search

ansys / pymapdl

Pythonic interface to MAPDL
https://mapdl.docs.pyansys.com
MIT License
419 stars 116 forks source link

build: bump the minimal group across 1 directory with 2 updates #3197

Closed dependabot[bot] closed 1 week ago

dependabot[bot] commented 1 week ago

Bumps the minimal group with 2 updates in the / directory: importlib-metadata and psutil.

Updates importlib-metadata from 7.1.0 to 7.2.0

Changelog

Sourced from importlib-metadata's changelog.

v7.2.0

Features

Commits


Updates psutil from 5.9.8 to 6.0.0

Changelog

Sourced from psutil's changelog.

6.0.0 2024-06-18

Enhancements

  • 2109_: maxfile and maxpath fields were removed from the namedtuple returned by disk_partitions()_. Reason: on network filesystems (NFS) this can potentially take a very long time to complete.
  • 2366_, [Windows]: log debug message when using slower process APIs.
  • 2375_, [macOS]: provide arm64 wheels. (patch by Matthieu Darbois)
  • 2396_: process_iter()_ no longer pre-emptively checks whether PIDs have been reused. This makes process_iter()_ around 20x times faster.
  • 2396_: a new psutil.process_iter.cache_clear() API can be used the clear process_iter()_ internal cache.
  • 2401_, Support building with free-threaded CPython 3.13.
  • 2407_: Process.connections()_ was renamed to Process.net_connections()_. The old name is still available, but it's deprecated (triggers a DeprecationWarning) and will be removed in the future.
  • 2425_: [Linux]: provide aarch64 wheels. (patch by Matthieu Darbois / Ben Raz)

Bug fixes

  • 2250_, [NetBSD]: Process.cmdline()_ sometimes fail with EBUSY. It usually happens for long cmdlines with lots of arguments. In this case retry getting the cmdline for up to 50 times, and return an empty list as last resort.
  • 2254_, [Linux]: offline cpus raise NotImplementedError in cpu_freq() (patch by Shade Gladden)
  • 2272_: Add pickle support to psutil Exceptions.
  • 2359_, [Windows], [CRITICAL]: pid_exists()_ disagrees with Process_ on whether a pid exists when ERROR_ACCESS_DENIED.
  • 2360_, [macOS]: can't compile on macOS < 10.13. (patch by Ryan Schmidt)
  • 2362_, [macOS]: can't compile on macOS 10.11. (patch by Ryan Schmidt)
  • 2365_, [macOS]: can't compile on macOS < 10.9. (patch by Ryan Schmidt)
  • 2395_, [OpenBSD]: pid_exists()_ erroneously return True if the argument is a thread ID (TID) instead of a PID (process ID).
  • 2412_, [macOS]: can't compile on macOS 10.4 PowerPC due to missing MNT_ constants.

Porting notes

Version 6.0.0 introduces some changes which affect backward compatibility:

  • 2109_: the namedtuple returned by disk_partitions()_' no longer has maxfile and maxpath fields.
  • 2396_: process_iter()_ no longer pre-emptively checks whether PIDs have been reused. If you want to check for PID reusage you are supposed to use Process.is_running()_ against the yielded Process_ instances. That will also automatically remove reused PIDs from process_iter()_ internal cache.
  • 2407_: Process.connections()_ was renamed to Process.net_connections()_. The old name is still available, but it's deprecated (triggers a

... (truncated)

Commits
  • 3d5522a release
  • 5b30ef4 Add aarch64 manylinux wheels (#2425)
  • 1d092e7 test subprocesses: sleep() with an interval of 0.1 to make the test process m...
  • 5f80c12 Fix #2412, [macOS]: can't compile on macOS 10.4 PowerPC due to missing MNT_...
  • 89b6096 process_iter(): use another global var to keep track of reused PIDs
  • 9421bf8 openbsd: skip test if cmdline() returns [] due to EBUSY
  • 4b1a054 Fix #2250 / NetBSD / cmdline: retry on EBUSY. (#2421)
  • 20be5ae ruff: enable and fix 'unused variable' rule
  • 5530985 chore(ci): update actions (#2417)
  • 1c7cb0a Don't build with limited API for 3.13 free-threaded build (#2402)
  • Additional commits viewable in compare view


Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore major version` will close this group update PR and stop Dependabot creating any more for the specific dependency's major version (unless you unignore this specific dependency's major version or upgrade to it yourself) - `@dependabot ignore minor version` will close this group update PR and stop Dependabot creating any more for the specific dependency's minor version (unless you unignore this specific dependency's minor version or upgrade to it yourself) - `@dependabot ignore ` will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself) - `@dependabot unignore ` will remove all of the ignore conditions of the specified dependency - `@dependabot unignore ` will remove the ignore condition of the specified dependency and ignore conditions
dependabot[bot] commented 1 week ago

The following labels could not be found: Maintenance, Dependencies.

codecov[bot] commented 1 week ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 84.47%. Comparing base (7c1eb1e) to head (43dc61c). Report is 14 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3197 +/- ## ========================================== - Coverage 86.63% 84.47% -2.17% ========================================== Files 52 52 Lines 9550 9596 +46 ========================================== - Hits 8274 8106 -168 - Misses 1276 1490 +214 ```
germa89 commented 1 week ago

minimum_requirements.txt always poses me the dilemma.... Update the requirements in this file, yes or no.

The goal of this requirements file is to specify a set of minimal libraries you need in order to send commands to MAPDL. It does not include plotting capabilities (depends on Matplotlib and Pyvista), advanced post-processing (PyMAPDL-Reader, Pandas, etc), etc. Just the bare minimum in order to send commands and get back their text output.

If we understand the word minimum as version dependent also... we should have then the minimum amount of libraries and the minimal version it works. We should use Numpy 1.22 instead of Numpy 2.0

If we understand the word minimum as only the minimal libraries, we could update the requirements in this file as the dependabot requires.

It should be mentioned that we do have CICD in place that tests for these cases: https://github.com/ansys/pymapdl/actions/runs/9610408523/job/26507001246?pr=3197

As I'm writing this, I lean towards updating the minimum_requirements.txt file. Just because the CICD tests this file and we should ensure compatibility with the newest versions. However I do not like to have such updates libraries in that file, when our pyproject.toml is not that strict.

Pinging @koubaa for feedback. Pinging @ansys/pyansys-core for awareness/feedback if appropriate.

GitHub
build: bump the minimal group across 1 directory with 2 updates · ansys/pymapdl@1deb00b
Pythonic interface to MAPDL. Contribute to ansys/pymapdl development by creating an account on GitHub.
RobPasMue commented 1 week ago

Hmm I see it like the two problems you pose...

Hope you get my point :)

GitHub
pyansys-geometry/.github/workflows/ci_cd.yml at b3001e7c325ec42c7bbc14fafcf25c622536fac6 · ansys/pyansys-geometry
A Python wrapper for Ansys Geometry Services. Contribute to ansys/pyansys-geometry development by creating an account on GitHub.
germa89 commented 1 week ago

@RobPasMue thank a lot you for your comment.

Regarding your first point... the issue is that the minimal requirements are a subset of the pyproject dependencies. For instance, pyvista is now part of the dependencies, but it is not actually needed to interact with MAPDL. By convention, in the pyproject toml the dependencies are the minimal dependencies, then doc or tests groups are installed on top of that. Following this approach, our dependencies should be the ones in minimal_requirements, and then have something like a default group which install the normal dependencies (pyvista, etc...). The issue is that you will have to install pymapdl as pip install 'ansys-mapdl-core[default]', which I dont really like to have to specify now a group.

Ideally, we should have another package ansys.mapd.minimal or similar... where the pyproject is filled with the minimal requirement and then pymapdl will depend on that. But that's a lot of work, for little gains. Or, having ansys.mapdl.pymapdl with all the libraries (pyvista, etc) and leave ansys.mapdl.core for the minimal. But again, big change.

Regarding your second point, I agree with that. But I'm not sure if it is reasonable to have pinned dependencies that never get updated.

germa89 commented 1 week ago

I think that taking the feedback from @RobPasMue , we should probably have a mix in minimun_requirements, which is having the minimal libraries and specify the minimal lower version that works:

ansys-api-mapdl==0.5.1
importlib-metadata>=4.0
numpy>=1.14.0
platformdirs>=3.6.0
psutil>=5.9.4
pyansys-tools-versioning>=0.3.3

The testing is going to be a bit "ramdon" because we are not enforcing any version, but I guess it is better than nothing?

RobPasMue commented 1 week ago

Yeah I get it... but, let's think about it from a general software perspective, not a PyMAPDL pov.

By definition, your dependencies in your pyproject.toml file are the required dependencies to run your library - the ones you need to at least run it, full stop. Following that approach, you might need to reduce the dependencies you are listing there. For convenience you are introducing dependencies that might be optional or nice-to-have. But really, what happens when you install ansys-mapdl-core is that all those "nice to have" libs are getting installed. So they are no longer "optional" really. You can only avoid installing them if you pass the --no-deps flag...

I would suggest that you have an all, default, extras target where you install the additional deps that are not strictly required. And within your library you can also inform whether the user needs to install a certain target of the library or not. You wouldn't be the first library to have this... PyPrimeMesh has a graphics target, PyAnsysGeometry has an all target, PyFluent has an extras target... What we should really do @ansys/pyansys-core is ask for homogeneity in that additional target 😄 another one for the bucket list.

The other approach you can take is to accept that your library has outgrown the minimal dependencies and pyvista is now a required dependency on your side. In that case, your current "dependencies" list would be fine but your minimum_requirements.txt file should include all of them.

germa89 commented 1 week ago

I totally agree.. the whole dependencies thing is bad organised in PyMAPDL. Pyvista should not have not been part of the required dependencies.

I'm quite conflicted about what to do. If have another library, if remove packages from the required dependencies, consider pyvista and the rest as required depencies.... or not doing anything at all! xD But eventually, this will have to be addressed in the future.

Let's wait for @koubaa opinion.

RobPasMue commented 1 week ago

. or not doing anything at all! xD

koubaa commented 1 week ago

@RobPasMue @germa89

We should have called it minimal_requirements. This is the minimal set of packages, not the minimum version of the full set of packages. The version numbers of the dependencies are not relevant here.

I'm open to some level of backwards incompatible changes at this stage to get this right, but I'm not sure what's best. To my pymapdl means a python client for mapdl, with all batteries included.

One option is to change the pypi package name of pymapdl to pymapdl and then ansys.mapdl.core can become the minimal package. To me that is what the term core already implies. Another option is to do a subpackage as German mentioned earlier.

germa89 commented 1 week ago

It is settled then.

In this file, we only care about libraries. I will do a follow up PR.

Regarding library splitting, it is going to be on the queue because it is not priority. But eventually, we might need to work on it.