Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Jun 24, 2024

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Jun 21, 2024

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 <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency
  • @dependabot ignore <dependency name> 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 <dependency name> 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 <dependency name> 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 <dependency name> will remove all of the ignore conditions of the specified dependency
  • @dependabot unignore <dependency name> <ignore condition> will remove the ignore condition of the specified dependency and ignore conditions

Bumps the minimal group with 2 updates in the / directory: [importlib-metadata](https://github.com/python/importlib_metadata) and [psutil](https://github.com/giampaolo/psutil).


Updates `importlib-metadata` from 7.1.0 to 7.2.0
- [Release notes](https://github.com/python/importlib_metadata/releases)
- [Changelog](https://github.com/python/importlib_metadata/blob/main/NEWS.rst)
- [Commits](python/importlib_metadata@v7.1.0...v7.2.0)

Updates `psutil` from 5.9.8 to 6.0.0
- [Changelog](https://github.com/giampaolo/psutil/blob/master/HISTORY.rst)
- [Commits](giampaolo/psutil@release-5.9.8...release-6.0.0)

---
updated-dependencies:
- dependency-name: importlib-metadata
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: minimal
- dependency-name: psutil
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: minimal
...

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot dependabot bot requested a review from a team as a code owner June 21, 2024 08:07
Copy link
Contributor Author

dependabot bot commented on behalf of github Jun 21, 2024

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

@dependabot dependabot bot requested review from germa89 and clatapie June 21, 2024 08:07
Copy link

codecov bot commented Jun 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

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

Additional details and impacted files
@@            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
Copy link
Collaborator

germa89 commented Jun 21, 2024

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
Pythonic interface to MAPDL. Contribute to ansys/pymapdl development by creating an account on GitHub.

@RobPasMue
Copy link
Member

RobPasMue commented Jun 21, 2024

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

  • Minimum dependencies as "the minimal amount of dependencies required to run the library": this you should just delegate it to pip to install your dependencies based on your pyproject.toml definition. For example, in pyansys-geometry we do the following to test this: https://github.com/ansys/pyansys-geometry/blob/b3001e7c325ec42c7bbc14fafcf25c622536fac6/.github/workflows/ci_cd.yml#L396-L442 - we basically pip install ansys-geometry-core[tests-minimal] - which leaves it up to pip to resolve our deps and installs pytest as well. We then run the tests against the resolved deps.
  • Minimal depencies as "the lowest version of your required dependencies that run PyMAPDL": this implies, IMO, having a reqs file, that you "never" update and that is frozen to the minimum version of your dependencies that you support (according to your pyproject.toml file) - in your case (and according to

    pymapdl/pyproject.toml

    Lines 15 to 38 in d061938

    dependencies = [
    "ansys-api-mapdl==0.5.1", # supports at least 2020R2 - 2022R1
    "ansys-mapdl-reader>=0.51.7",
    "ansys-math-core>=0.1.2",
    "ansys-platform-instancemanagement~=1.0",
    "platformdirs>=3.6.0",
    "click>=8.1.3", # for CLI interface
    "tabulate>=0.8.0", # for cli plotting
    "grpcio>=1.30.0", # tested up to grpcio==1.35
    "importlib-metadata>=4.0",
    "matplotlib>=3.0.0", # for colormaps for pyvista
    "numpy>=1.14.0,<1.25.0; python_version < '3.9'",
    "numpy>=1.14.0,<2.0.0; python_version >= '3.9'",
    "pexpect>=4.8.0 ; platform_system=='Linux'",
    "protobuf>=3.12.2", # minimum required based on latest ansys-grpc-mapdl
    "psutil>=5.9.4",
    "pyansys-tools-versioning>=0.3.3",
    "ansys-tools-path>=0.3.1",
    "pyiges[full]>=0.3.1", # Since v0.3.0, the 'full' flag is needed in order to install 'geomdl'
    "pyvista>=0.38.1",
    "scipy>=1.3.0", # for sparse (consider optional?)
    "tqdm>=4.45.0",
    "vtk>=9.0.0",
    ]
    ) this would be something like:
 ansys-api-mapdl==0.5.1
ansys-mapdl-reader==0.51.7
ansys-math-core==0.1.2
ansys-platform-instancemanagement==1.0.0
platformdirs==3.6.0
...
grpcio==1.30.0
...
numpy==1.14.0
...

Hope you get my point :)

GitHub
A Python wrapper for Ansys Geometry Services. Contribute to ansys/pyansys-geometry development by creating an account on GitHub.

@germa89
Copy link
Collaborator

germa89 commented Jun 21, 2024

@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
Copy link
Collaborator

germa89 commented Jun 21, 2024

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
Copy link
Member

RobPasMue commented Jun 21, 2024

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
Copy link
Collaborator

germa89 commented Jun 21, 2024

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
Copy link
Member

. or not doing anything at all! xD

@koubaa
Copy link
Contributor

koubaa commented Jun 21, 2024

@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
Copy link
Collaborator

germa89 commented Jun 24, 2024

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.

@germa89 germa89 merged commit 2b9ba18 into main Jun 24, 2024
41 checks passed
@germa89 germa89 deleted the dependabot/pip/minimal-0edef2a80a branch June 24, 2024 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants