-
Notifications
You must be signed in to change notification settings - Fork 251
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 LegacyVersion/LegacySpecifier #321
Comments
We should probably figure out how many projects this would make uninstallable on PyPI before we decide to do this. |
Without necessarily deciding on a deprecation period, simply poping up a Warning when |
I agree with @dstufft that a quick analysis of the latest releases of projects to see which ones would get excluded would be good. But I will also say that the consistent confusion about |
Q: Who's doing the analysis tho? |
I did the analysis (scripts and data are here). Out of nearly 2M releases on PyPI, less than 0.2% releases have a The last release to publish with a |
Thanks @di! I say we can deprecate now (say, in a release this month?) and drop support in early 2021. |
Do you happen to have the spare cycles to do the same thing with specifiers @di? Just using the info that's in the eDB should be good enough. The concern is that it doesn't surprise me that versions are a minor %, given PyPI has actively rejected non PEP 440 versions for awhile, so anything that has been released recently is incapable of not being a valid PEP 440 version. However, I don't think we do the same validation on dependency specifiers, which means people could be publishing invalid specifiers still to this day. |
@xavfernandez @pradyunsg Regarding a deprecation warning, I'm thinking that we might want to handle this differently when deprecating in this library vs. deprecating in pip. Here, I'd expect any time a But I'm not super familiar w/ pip's internals, so maybe that isn't necessary? |
FWIW, I realized after reading your comment that we'd probably want to expose this warning in some form to end users of pip. I was thinking purely in terms of deprecating in this library. :)
I think we'd be putting this through the
Huh. TIL. I'd argue: if warehouse isn't performing validation on specifiers, maybe we should treat these as two separate deprecations? :) |
I parsed all the specifiers as well (scripts are here, GitHub couldn't handle the CSV file) Of the nearly 5M requirement strings, only 5719 parsed as including a In addition, a lot of these look like typos, e.g. |
Cool, that strongly suggests that deprecating and ultimately removing these things is not going to cause any major problems. The only other comment I have is it might make sense to keep the classes around, but simply stop the fallback to the Legacy classes when parsing. That would mean that by default we would fail on legacy versions and specifiers, but if someone is using the API directly, and they have a reason to, they can still interact with legacy versions/specifiers. I don't believe either of these modules really requires much in the way of maintenance, so I don't think it would be a large maintenance burden or anything. The main reason I could see not to do that, is specifically for the specifier code, is that there are some minor simplifications that could be made to the logic if we completely drop the legacy forms. |
I say drop the old versions and if people really need them they can either copy the code or rely on an old version of 'packaging'. |
I agree with @brettcannon. So, if no one is opposed, uhm, let's deprecate both the things in packaging this month, scheduling for removal in early 2021? I'm kinda thinking of 21.0 as a "drop legacy cruft" releases for both pip and packaging, although in this case, we could definitely get away with a shorter deprecation period. |
I suppose I can always handle this another way, but for what it's worth, I use this package primarily to handle version checking of non-Python projects used by Python code. Arguably, this isn't the point of this package and I shouldn't have relied on this package to do this in the first place, but it's nice to have a generic way of dealing with version strings in Python. I'm not sure how many other people out there do something similar, but at least for me, this package isn't solely useful for looking at version strings on PyPI. I for one would be perfectly happy to see PEP440 and "legacy" versions fully-separated from each other (in fact, I already do this). However, I can see the argument that "legacy" (or "non-Python") version handling doesn't really belong in the Python Of course, I understand if the PyPA team doesn't want to add the maintenance burden of creating a new |
Thanks for sharing @jimporter. I think unfortunately that's a use case that this project isn't able to support. That said, the license would permit pulled out the |
@di Ok, thanks for the info. Do you foresee any major changes to the APIs for the PEP440-style versions? At a quick glance, it shouldn't be too difficult to provide an API-compatible package that solely provides "legacy" versioning; then, it'd be easy to use one or the other or both, as needed. In fact, fully-separating PEP440 versions and legacy versions would probably make both implementations a bit simpler: That said, I'm not sure yet what I'll do here; maybe for my purposes, it'd be better to parse versions differently from how |
It's unlikely there are any major changes to the |
Another example: #341 |
3 months is enough time, yea? Let's deprecate stuff in master now, and drop them in a 21.0 release. I like the idea of starting 2021 with a nicer situation. |
https://pypi.org/project/setuptools-scm generated local versions parse as a legacy, see #356. So this change might impact local development, though not published packages. |
In version 22.0, `packaging` dropped `LegacyVersion`, which we rely on. See pypa/packaging#321
In version 22.0, `packaging` dropped `LegacyVersion`, which we rely on. See pypa/packaging#321
pypa/packaging#321 removed support for parsing legacy PyPI versions, which we needed to make our version enumeration/sorting work. This upgrade happened in f968260, but somehow we didn't notice this breakage.
pypa/packaging#321 removed support for parsing legacy PyPI versions, which we needed to make our version enumeration/sorting work. This upgrade happened in f968260, but somehow we didn't notice this breakage.
- this release does not add any new functionality nor modify existing functionality - **SUMMARY** - see commit 21bd027 for the initial attempt at fixing the upload error - this change fixed the upload error, but changed the functionality of `python_requires` since any `3.0.N` version of python would become incompatible with this change - see commit d3e02a7 for the proper fix to the original upload error while maintaining compatibility for any `3.0.N` version of python - **EXPLANATION (taken from pull request thread)** After doing some digging, this is the likely culprit for what caused this problem: - pypa/packaging#407 - which was the result of pypa/packaging#566 (related: pypa/packaging#530 and pypa/packaging#321) - which in turn looks like the result of the discussion at https://discuss.python.org/t/how-to-pin-a-package-to-a-specific-major-version-or-lower/17077/8 It looks like this is the expected behavior as defined in PEP 440 under the [Inclusive ordered comparison section](https://peps.python.org/pep-0440/#inclusive-ordered-comparison): > An inclusive ordered comparison clause includes a comparison operator and a version identifier, and will match any version where the comparison is correct based on the relative position of the candidate version and the specified version given the consistent ordering defined by the standard [Version scheme](https://peps.python.org/pep-0440/#version-scheme). Following the link to the [Version scheme](https://peps.python.org/pep-0440/#version-scheme) section and looking at the specification under the [Public version identifiers](https://peps.python.org/pep-0440/#public-version-identifiers) section: > The canonical public version identifiers MUST comply with the following scheme: > `[N!]N(.N)*[{a|b|rc}N][.postN][.devN]` > Public version identifiers MUST NOT include leading or trailing whitespace. > > Public version identifiers MUST be unique within a given distribution. > ... The last line included above seems to be the "loose implementation" of the version modifier that the issues and pull requests I linked to at the very top were discussing ("After doing some digging, this is the likely culprit for what caused this problem"). Once that "loose implementation" was fixed, any package that didn't follow the PEP 440 specification for version identifiers broke. In this package, the break occurred because of the `>=3.0.*` comparison, which IS NOT unique within a given distribution, as opposed to `>=3` (what commit d3e02a7 does), which IS unique within a given distribution. To clarify: it looks like the problem we see in this issue is because of implementation fixes in the packaging tools to more closely follow PEP 440, specifically **"Public version identifiers MUST be unique within a given distribution."** Any package that relied on the previous implementation that loosely verified the PEP 440 specification but did not strictly follow PEP 440 broke after the implementation of the packaging tool(s) were fixed to more closely follow PEP 440. More explicitly (from [this comment](https://discuss.python.org/t/how-to-pin-a-package-to-a-specific-major-version-or-lower/17077/8) on the [How to pin a package to a specific major version or lower](https://discuss.python.org/t/how-to-pin-a-package-to-a-specific-major-version-or-lower/17077) discussion): > Christopher already made the response I was going to make: for PEP 440 as written, using wildcards outside of “==” and “!=” comparisons isn’t valid. > > Allowing them for “>=” and “<=” would be reasonable, but it would involve a PEP to fix the spec. (It wasn’t a conscious choice to exclude them, we just didn’t notice at the time that the inclusive ordered comparison operators weren’t formally defined as combining an exclusive ordered comparison with a version match, so the tools have been written to ignore the wildcard instead of paying attention to it) > > Making a coherent definition wouldn’t be too hard: just ignore the wildcard for the exclusive ordered comparison part and keep it for the version matching part. Here are some other posts that aren't directly relevant, but might be interesting tangents for anyone interested in packaging problems: - https://stackoverflow.com/questions/19534896/enforcing-python-version-in-setup-py - https://packaging.python.org/en/latest/guides/distributing-packages-using-setuptools/#python-requires - https://packaging.python.org/en/latest/guides/distributing-packages-using-setuptools/#package-data - https://setuptools.pypa.io/en/latest/userguide/datafiles.html - https://peps.python.org/pep-0345/#requires-python - https://stackoverflow.com/questions/8795617/how-to-pip-install-a-package-with-min-and-max-version-range - https://python3statement.org/practicalities/ - https://discuss.python.org/t/requires-python-upper-limits/12663/20 - https://stackoverflow.com/questions/13924931/setup-py-restrict-the-allowable-version-of-the-python-interpreter/13925176#13925176
There's too much confusion about how the
LegacyVersion
andLegacySpecifier
interact with their non-legacy counterparts when mixed. (See #74, #112, #275, #307, #320, and probably more)In #12 @dstufft said :
But that was six years ago. I think the community has been publishing/using non-legacy versions for long enough that we can remove the need for this library to support it.
I propose a deprecation period, where creating a
LegacyVersion
/LegacySpecifier
produces a deprecation warning, followed by a removal, where this would raise anInvalidVersion
/InvalidSpecifier
exception instead (possibly with some additional details about them being legacy).The text was updated successfully, but these errors were encountered: