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

Deprecate the method aiida.get_strict_version #5512

Merged
merged 1 commit into from
May 12, 2022

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented May 3, 2022

The method uses the distutils package of the standard library. With
the acceptance of PEP 632, this module was deprecated in Python 3.10 and
it will be removed in Python 3.12.

Since there is no exact replacement of the StrictVersion class in
another package, we deprecate the method and slate it for removal in the
next major version. We could have used packaging.version.Version as a
replacement, but the method is not used internally and doesn't seem to
be used by plugin packages either.

The method uses the `distutils` package of the standard library. With
the acceptance of PEP 632, this module was deprecated in Python 3.10 and
it will be removed in Python 3.12.

Since there is no exact replacement of the `StrictVersion` class in
another package, we deprecate the method and slate it for removal in the
next major version. We could have used `packaging.version.Version` as a
replacement, but the method is not used internally and doesn't seem to
be used by plugin packages either.
Copy link
Member

@ramirezfranciscof ramirezfranciscof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes are straightforward and look good to me 👍. Now, I understand the method was originally introduced (#1686) to deal with this comment (#1685):

Also, can we consider to implement some kind of version_info or similar that returns some standardized version number, for instance like a tuple.

I'm not 100% sure exactly how StrictVersion standardized this, but the returned value of get_version / __version__ seems to be the same (format, not value) as what it was back then, so it is not immediate to me what changed or why the need for a more standardized output of the version was relevant then but not now. Maybe you can add some justification for this in the commit message?

@sphuber
Copy link
Contributor Author

sphuber commented May 10, 2022

so it is not immediate to me what changed or why the need for a more standardized output of the version was relevant then but not now. Maybe you can add some justification for this in the commit message?

Honestly, I also don't see it. I realize that I added it back then, but I don't remember why I thought that was a good idea. The standard for Python packages is to provide a __version__ attribute that has the package's version number in string form. We make sure that this is PEP 440 compatible. If someone wants to have this in a form that can easily access parts of the identifier, they can do that themselves trivially with

from aiida import __version__
from packaging.version import Version
Version(__version__)

I don't see why we should provide a method for this.

But let's ask @espenfl who requested the feature at the time. Do you still use aiida.get_strict_version?

@sphuber
Copy link
Contributor Author

sphuber commented May 12, 2022

In any case, the get_strict_version has to be removed, since the distutils package will be removed in Python 3.12 and just changing the return type of the method also technically breaks the interface. Since there is an easy replacement, I think we can merge this.

@sphuber sphuber merged commit abc735a into aiidateam:main May 12, 2022
@sphuber sphuber deleted the fix/5511/remote-distutils branch May 12, 2022 12:15
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.

2 participants