Dependency upgrade/cleanup after dropping py3.9#7245
Conversation
156e311 to
ab9c709
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7245 +/- ##
==========================================
+ Coverage 79.85% 79.86% +0.01%
==========================================
Files 566 566
Lines 43963 43958 -5
==========================================
Hits 35103 35103
+ Misses 8860 8855 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Consider also this #7240 (comment) |
|
Consider #7240 (comment) |
728a4c5 to
07373ba
Compare
|
Consider upgrading pytest_asyncio https://github.com/aiidateam/aiida-core/pull/7258/changes#r2889517642 |
Let's do that in a separate PR, last time I tried it I ran into some ugly issues. |
| UnionType = types.UnionType | ||
|
|
||
| # Fallback for Python 3.9 and older | ||
| from typing_extensions import ParamSpec |
There was a problem hiding this comment.
One more thing like this: Move TypeAlias import from typing_extensions to typing in aiida/tools/graph/age_entities.py
There was a problem hiding this comment.
So I only changed it in src/aiida/engine/processes/functions.py and not in the other file. There will be a larger PR where we will use pyupgrade to automatically update every source file to 3.10 syntax and this will include also changes as removing typing_extensions imports if it is not done automatically by the tool. We want to do this right before the next minor release because it would create a lot of annoying diff noise resulting in a loft of merge conflicts for the patch releases of 2.8.x that still supports 3.9. In src/aiida/engine/processes/functions.py we already changed this part of the code so it will probably anyway result in a merge conflict. Hope this is okay.
`get-annotations` was a backport needed for Python <3.10. Use `inspect.get_annotations` from the stdlib instead. Run pre-commit to auto-fix TOML formatting in uv.lock and regenerate environment.yml after dependency changes. Co-authored-by: Daniel Hollas <danekhollas@gmail.com>
Replace `importlib_metadata` backport with `importlib.metadata` from the standard library, which is sufficient for the supported Python versions (>=3.10). Remove now-unnecessary `# type: ignore[no-untyped-call]` comments on `EntryPoint.load()` calls since the stdlib version is fully typed. Fix `eps()` to flatten `SelectableGroups` (dict of group name to `EntryPoints`) returned by `entry_points()` into a flat sorted `EntryPoints` list, since the stdlib `entry_points()` returns a dict-like `SelectableGroups` rather than a flat iterable. Fix `EntryPoints` construction in test fixtures to use list unpacking instead of tuple concatenation, resolving mypy overload resolution errors.
Regenerate uv.lock and environment.yml via pre-commit hooks after bumping asyncssh version constraint.
07373ba to
d3a4e57
Compare
`get-annotations` was a backport needed for Python <3.10. Use `inspect.get_annotations` from the stdlib instead. Co-authored-by: Daniel Hollas <danekhollas@gmail.com>
Replace `importlib_metadata` backport with `importlib.metadata` from the standard library, which is sufficient for the supported Python versions (>=3.10). Remove now-unnecessary `# type: ignore[no-untyped-call]` comments on `EntryPoint.load()` calls since the stdlib version is fully typed. Flatten `SelectableGroups` returned by `entry_points()` on Python <3.12 via `dict.values()` to get a flat list of `EntryPoint` objects, since `SelectableGroups` iterates over group name strings, not entry points. Fix `EntryPoints` construction in test fixtures to use list unpacking instead of tuple concatenation, resolving mypy overload resolution errors.
Regenerate uv.lock and environment.yml via pre-commit hooks after bumping asyncssh version constraint.
d3a4e57 to
d2a495d
Compare
| all_eps = entry_points() | ||
| return EntryPoints(sorted(all_eps, key=lambda x: x.group)) | ||
| eps = entry_points() | ||
| # Python <3.12 returns SelectableGroups (dict subclass); flatten it. |
There was a problem hiding this comment.
We have to sadly adapt it
https://docs.python.org/3.12/library/importlib.metadata.html
importlib_metadata 5.0 and Python 3.12, entry_points always returns an EntryPoints object.
But I think its worth removing a dependency
`get-annotations` was a backport needed for Python <3.10. Use `inspect.get_annotations` from the stdlib instead. Co-authored-by: Daniel Hollas <danekhollas@gmail.com>
Replace `importlib_metadata` backport with `importlib.metadata` from the standard library, which is sufficient for the supported Python versions (>=3.10). Handle the `entry_points()` return type difference across Python versions using `sys.version_info` guard: Python <3.12 returns SelectableGroups (dict subclass iterating group names), while >=3.12 returns EntryPoints directly. Remove now-unnecessary `# type: ignore[no-untyped-call]` comments on `EntryPoint.load()` calls since the stdlib version is fully typed. Fix `EntryPoints` construction in test fixtures to use list unpacking instead of tuple concatenation, resolving mypy overload resolution errors.
Regenerate uv.lock and environment.yml via pre-commit hooks after bumping asyncssh version constraint.
d2a495d to
7b3d00b
Compare
| else: | ||
| # Python <3.12: entry_points() returns SelectableGroups (dict subclass) | ||
| # that iterates group names, not EntryPoints. Flatten via .values(). | ||
| all_eps = [ep for group_eps in entry_points().values() for ep in group_eps] |
There was a problem hiding this comment.
Had to iterate through this logic multiple times. I think this is the clearest way. Its a bit annoying but I think its worse the dependency drop.
|
We might just keep this PR stale until we plan the minor release. After going through all changes I feel like the changes are not really needed for any subsequent changes so there is no rush for merging this and it will make our life just harder for patch releases. |
Agreed on principle, but at the same time, these changes seem quite localized, and seem unlikely to induce conflicts with other PRs? So I'd be for merging this. If you plan to leave this stale, than I'd propose at the very least merge a separate PR with the |
Also true, but I thought that the uv changes might be actually annoying to merge for patch releases, on the other hand these conflicts can be fixed by the pre-commit hook. And touching a PR after some months is also often almost like starting from scratch. I don't have a strong opinion on this, we can also merge now. Your call. |
Yeah, but that would mean you'd have to block all PRs that touch dependencies. And as you say, conflicts in uv.lock can be generally solved by regenerating the uv.lock (which shouldn't really matter in terms of patch version release, since the uv.lock only influences what gets tested in CI. I'd say on the opposite side, the uv.lock should be merged especially, since otherwise every PR that makes a small change in dependency might get this big diff. Also, merging only just before the minor version release is riskier since the change have not been tested on main for a while. So overall I feel this should be merged now. |
`get-annotations` was a backport needed for Python <3.10. Use `inspect.get_annotations` from the stdlib instead. Co-authored-by: Daniel Hollas <danekhollas@gmail.com>
Replace `importlib_metadata` backport with `importlib.metadata` from the standard library, which is sufficient for the supported Python versions (>=3.10). Handle the `entry_points()` return type difference across Python versions using `sys.version_info` guard: Python <3.12 returns SelectableGroups (dict subclass iterating group names), while >=3.12 returns EntryPoints directly. Remove now-unnecessary `# type: ignore[no-untyped-call]` comments on `EntryPoint.load()` calls since the stdlib version is fully typed. Fix `EntryPoints` construction in test fixtures to use list unpacking instead of tuple concatenation, resolving mypy overload resolution errors.
|
Thanks a lot, @agoscinski! Great work! Sry was a bit too late on that one ^^ |

Removing the two dependencies
importlib-metadataandget-annotationsthat are not anymore needed since we dropped py3.9, and updateasyncsshto a version that supports py3.9.We could also replace
pytzwithzoneinfobut that affects the hash function because the binary representation is not the same. A database migration seems a bit overkill to drop one dependency.