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

parse(">3.6") >= parse(">=3") is False #591

Closed
Torxed opened this issue Sep 13, 2022 · 18 comments
Closed

parse(">3.6") >= parse(">=3") is False #591

Torxed opened this issue Sep 13, 2022 · 18 comments

Comments

@Torxed
Copy link

Torxed commented Sep 13, 2022

I'm wondering if my assumptions are wrong here, or if this is unintentional behavior?

from packaging.version import Version, parse as VersionParser

print(VersionParser('>3.6') > VersionParser('>=3'))
print(VersionParser('>=3.6') > VersionParser('>=3'))

I would assume that anything greater than 3.6 is also greater than 3.
But in this case the result will be False followed by True if I instead use >=.

But if you're parsing other peoples packages, from pypi.org for instance you're not really in a position to alter from > to >=.
Hope the example is clear and maybe it's just my assumption that's off.

  • packaging 21.3
  • Python 3.10.5
@Torxed
Copy link
Author

Torxed commented Sep 13, 2022

Testing straight against the main branch I noticed that the version definition >3.6 and >=3.6 raises:

packaging.version.InvalidVersion: Invalid version: '>3.6'

This will be interesting to witness on the pypi.org packaging system.
Out of curiosity, how would you define all versions above 3.6 to be supported if you can't use >= in the definition?

Programmatically I get that you can do 3.6 >= X but won't that be hard to define in the packaging definition?
Or do we have to define all versions one by one, including patch versions?
And what about versions that break stuff, where there might be packaging versions >=3.6, !3.6.4, !3.8 to deal with breaking Python releases on a specific package.

@pradyunsg
Copy link
Member

You want to use specifier set instead of version, for parsing this form.

@Torxed
Copy link
Author

Torxed commented Sep 13, 2022

You want to use specifier set instead of version, for parsing this form.

They are not comparable tho.
Take this for instance:

SpecifierSet('>=3.6') >= SpecifierSet('>=3')

TypeError: '>=' not supported between instances of 'SpecifierSet' and 'SpecifierSet'

And sure, you could use SpecifierSet('>=3.6').filter("3") but then you'd need to extract the version and omit key information from the comparing object. Or is there a workaround here too?

I'll add to this problem of comparing, that it doesn't support

SpecifierSet('>=3.6') ^ SpecifierSet('>=3')

Which would have solved this use case as I could do any(SpecifierSet('>=3.6') ^ SpecifierSet('>=3')) to eliminate all possible combinations in some clever ways. But it only has & (AND) for combining. Which won't help here.

@uranusjr
Copy link
Member

I don’t think comparison between SpecifierSet instances makes sense. Set operations may make sense (it needs to return something else e.g. SpecifierSetCollection because it’s impossible to serialise | back to string) but it’s also valid to argue this is out of scope for packaging. You can easily just do the or in code. Maybe we even shouldn’t have implemented & in the fist place to avoid false user expectations like this.

@Torxed
Copy link
Author

Torxed commented Sep 13, 2022

My case that I'd like to argue here, is that the packaging.version should handle allowed formats with pypi.org in mind and that packaging should allow (as it has in the past) comparison between two version definitions.
I generally don't care about the how as long as there is a way. But currently I'm locked into a corner where I have to do string processing myself, which I would assume this library is meant to do.

And I would assume that comparing equal objects is within the scope, given that they follow the right format [!<>=][0-9.]+ (wild pseudo regex, but hopefully you get the idea).
I can barely find a workaround to this issue without ignoring the use packaging.* or ignoring key information in the value.
And it feels like the reason I stopped using distutils.version.LooseVersion is happening here too. Which feels weird and unreliable.

I'm getting the impression that I'm wild and crazy for suggestion this to be sane:

SpecifierSet('>=3.6') >= SpecifierSet('>=3')

How would people generally compare two version statements?
One would be the library and the other the operating systems requirements for instance.


Just to clarify, I get that this is a "workaround" to the above statement.

SpecifierSet('>=3.6').contains("3.10")

But it's also not the same. As you would have to iterate every version aspect imagined within >=3.
So it's not exactly a 1:1 in functionality.

@pradyunsg
Copy link
Member

What are you trying to do? Why do you want to compare these strings and what is your expectation for how the comparison would work?

@Torxed
Copy link
Author

Torxed commented Sep 13, 2022

I'm comparing a supported version of python defined by the operating system which happens to be 3.7 - 3.10 (So essentially anything >=3.7).
I'm also taking user input to override the above mentioned use case.
This is then used when mirroring pypi.org among other things where the user can define to only mirror packages that are --py-ver >=3.

The second use case is that of using it for general package management in the operating system I'm using.
That case I could argue is an issue for the operating system and use whatever they implement.

tl;dr: Take package cms-figure for instance, it defines "requires_python": ">=2.7, <3", and I'd like to define a supported version set of >=3 and now compare those before choosing to download the package or not. In this case not because >=3 is not within >=2.7 and <3.


My expectation is as I stated in my first post:

VersionParser('>=3.6') > VersionParser('>=3') # == True

And not what's currently happening, which is incorrect since 3.6 is still greater than 3 but we're getting the opposite.

VersionParser('>3.6') > VersionParser('>=3') # == False

As described above by others. This only works on the current stable release, on future versions this will fail and you need to use SpecifierSet instead which lacks the comparison operator all together. And there's no feature to do the above.
And it lacks sufficient set operators for bitwise operations. a XOR for instance would have been useful if you expect a set to behave like a set.

@layday
Copy link
Member

layday commented Sep 13, 2022

None of these are versions; they are version constraints. The only reason this doesn't raise is that the version's being parsed as a LegacyVersion, which as I understand will be removed in the next version (of packaging). What does it even mean to compare version constraints -- are you trying to find which one has the lowest boundary, the highest boundary, the broadest range? Is >3.6 larger than >3? Is >3.6 larger than >3.7,<4?

@Torxed
Copy link
Author

Torxed commented Sep 13, 2022

Is >3.6 larger than >3.7,<4?

No. Partially yes but strictly no.

>3.6 is not larger than >3.7 in the statement >3.7, <4 and is therefor False, partially however 3.7+ is within <4.
Some use cases does not have a highest boundary, some do. Some do not have a lowest boundary, some do.

>3.6 larger than >3

Yes, as the comparison is if any version above 3.6 is greater than any version above 3.
Unpacking >3.6 would mean 3.7 + 3.8 + 3.9 etc (including patch version, but since neither defines that granularity that should be considered ignored). And Unpacking >3 would mean 3.1 + 3.2 + 3.3 etc.
Thus, all of >3.6 is greater than >3.
If there were an upper boundary or highest boundary as you put it the context would change and different rules apply.

None of these are versions; they are version constraints.

Sure, but constraints or versions - I'd still expect to be able to compare equal object types in this instance. (I'd also expect Version("3.5") in SpecifierSet(">=3") to work as the two types are within the packaging domain but that does not work either)

@layday
Copy link
Member

layday commented Sep 13, 2022

Thus, All of >3.6 is greater than >3.

All numbers greater than 3.6 are greater than 3. But greater than 3.6 is not greater than greater than 3; ⊃ does not mean > unless you define the ordering of sets in such terms.

Version("3.5") in SpecifierSet(">=3")

It does work.

@Torxed
Copy link
Author

Torxed commented Sep 13, 2022

It does work.

Apologies, it didn't work earlier on two other machines. Works locally on a third and I'm not sure what the difference is at this moment but earlier it required a str or bytes like on the left side. Either way, main opinion still stands.

@layday
Copy link
Member

layday commented Sep 13, 2022

It's an interesting idea but set operations for version specifiers are not formalised. Your best bet's simply to take a min version and a max version as input instead of a version specifier.

@brettcannon
Copy link
Member

I understand the desire behind the feature request, but Python packaging has a separate concept for determining whether a specific version (packaging.version) meets some version constraint (packaging.specifiers). I don't think there enough of a need here for us to implement and support such comparison support. Obviously please feel free to implement this and post it to PyPI for others to use.

@brettcannon brettcannon closed this as not planned Won't fix, can't repro, duplicate, stale Sep 23, 2022
@Torxed
Copy link
Author

Torxed commented Dec 2, 2022

@brettcannon sorry for the late answer, but I've been trying to twist and turn on your claim that there is a separate concept implying a solution to this problem. Specifically the statement:

Python packaging has a separate concept for determining whether a specific version (packaging.version) meets some version constraint (packaging.specifiers).

As you did not point to a specific solution, some attempts:

>>> packaging.specifiers.Specifier('>=3').contains('>3.6')
False
>>> list(packaging.specifiers.Specifier('>=3').filter(['>3.6']))
[]

There doesn't seem to be support for ranges anywhere on these objects you propose.
Obviously packaging.version.Version (being an absolut version) would work against packaging.specifiers.Specifier as this is the most common use case.
However, if >3.6 translated to a VersionRange object or something similar with support for min() (and max(x) if it's a <=) I could easily see support for the above use case by extracting min() to get a packaging.version.Version (or LegacyVersion). But that's currently not possible from my understanding.

Or if LegacyVersion supported the magic functions for __gt__ etc. Then that would be a shortcut, but perhaps more of a hack than dealing with ranges appropriately by giving support to Specifier(...).contains() to deal with the right operator being a Specifier as well.

All the tests I could find:

# Test the greater than equal operation
("2.0", ">=2"),
("2.0", ">=2.0"),
("2.0", ">=2.0.0"),
("2.0.post1", ">=2"),
("2.0.post1.dev1", ">=2"),
("3", ">=2"),

Are done on a fixed Version against a Specifier or similar setup. There's never a range-on-range check done anywhere? And I cannot find any examples anywhere on how this would be done. So I've gone through a lot of the source code trying to figure out a way to expand or iterate a Specifier into individual Version. All roads lead to, I don't think your statement is correct in terms of range-on-range situations, and there is no "concert for determining" a range-on-range.

I think I've wasted a great deal of time trying to investigate the current concept by thinking there was a solution.


Would a PR be welcome to introduce the min and max operators on Specifiers?
Or would the logic be well placed in .contains() and .filter()?
I have the same amount of spare time as you probably so obviously I would if I could. And if my above assumption is correct, I can see if I can find the time to implement it. Otherwise I'll just bail to third party libraries as I have more pressing issues with python tooling, the build ecosystem of Python and other packaging issues that's been pestering me for a few versions now.

@pradyunsg
Copy link
Member

pradyunsg commented Dec 2, 2022

Ah, this is the stupidness1 thanks to legacy version:

>>> from packaging.version import *
>>> Version(">3.6")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/pradyunsg/Developer/github/pip/.venv/lib/python3.10/site-packages/packaging/version.py", line 266, in __init__
    raise InvalidVersion(f"Invalid version: '{version}'")
packaging.version.InvalidVersion: Invalid version: '>3.6'
>>> parse(">3.6")
<LegacyVersion('>3.6')>

That's been removed in #530, so the behaviour on main is now:

>>> from packaging.version import *
>>> parse(">3.6")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/pradyunsg/Developer/github/packaging/packaging/version.py", line 52, in parse
    return Version(version)
  File "/Users/pradyunsg/Developer/github/packaging/packaging/version.py", line 197, in __init__
    raise InvalidVersion(f"Invalid version: '{version}'")
packaging.version.InvalidVersion: Invalid version: '>3.6'

Adding an example from what you tried:

>>> import packaging.specifiers
>>> packaging.specifiers.Specifier('>=3').contains('>3.6')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/pradyunsg/Developer/github/packaging/packaging/specifiers.py", line 552, in contains
    normalized_item = _coerce_version(item)
  File "/Users/pradyunsg/Developer/github/packaging/packaging/specifiers.py", line 25, in _coerce_version
    version = Version(version)
  File "/Users/pradyunsg/Developer/github/packaging/packaging/version.py", line 197, in __init__
    raise InvalidVersion(f"Invalid version: '{version}'")
packaging.version.InvalidVersion: Invalid version: '>3.6'

Footnotes

  1. i.e. the code has stupid/unexpected behaviours.

@brettcannon
Copy link
Member

There doesn't seem to be support for ranges anywhere on these objects you propose.

Correct. My comment was strictly addressing the fact you can't compare versions like you were trying and that we just have a concept of whether a version fits into a specifier(s), not that there was some solution to ranges. Sorry if that's the impression you got.

Would a PR be welcome to introduce the min and max operators on Specifiers?

I personally don't see enough of a need for this functionality for such a thing to live in this package. We purposefully try to keep this project small as it's just two of us maintaining it and it's trying to cover all the low-level packaging standards. Since this isn't necessary for e.g. pip to determine whether it can install something, I think it's a better fit for some other project.

@Torxed
Copy link
Author

Torxed commented Dec 3, 2022

I personally don't see enough of a need for this functionality for such a thing to live in this package. ... I think it's a better fit for some other project.

I would kindly suggest not to encourage contributions if the contribution isn't actually worthwhile.
Maybe it was a language barrier issue on my part, but I interpreted this as a welcome change:

feel free to implement this and post it to PyPI

Unless you meant another project within PyPi, in which case perhaps point towards what project would welcome it.

In any case, I still believe versioning ranges should be allowed as a comparison. Maybe not for pip but since the versioning scheme is very loose in Python packaging, delivering this type of functionality to validate version ranges within packages to distribution maintainers and application maintainers would be helpful. Normal users wouldn't need this, I agree. But there are others that do who help package and develop certain things which boosts the community and involvement and ultimately hopefully increase contributors.

I don't expect anyone else to implement complex version checks in their specific applications when it's tied to the chosen data structure by PyPi, much like I wouldn't expect anyone to implement version check for pacman (ArchLinux Package Manager) when there's libalpm.

With that said, not all features have to be accepted and implemented. It's entirely up to you the maintainers. If you feel this strongly about it I'm happy to let it go. But know that it would have helped to offload pypi.org in a sense and the desire to have it (although not among the masses) is still there :)

@brettcannon
Copy link
Member

feel free to implement this and post it to PyPI

Unless you meant another project within PyPi,

Yes, I meant another project on PyPI. If I had wanted to suggest we would consider the functionality I would have said something like, "please submit a PR and we will be happy to review/consider it" or something like that where I reference us as a project.

in which case perhaps point towards what project would welcome it.

I honestly have no idea as there isn't any "higher-level utility library for packaging-related things" project that I'm aware of; most projects at that level of end-user tools and have code to meet their own needs. I personally would consider creating your own project for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants