Opened 5 years ago

Last modified 3 years ago

#3403 assigned task

Submit patch/bug report to Python/pip about problematic version number checking

Reported by: pett Owned by: Zach Pearson
Priority: major Milestone:
Component: Build System Version:
Keywords: Cc: chimera-programmers
Blocked By: Blocking:
Notify when closed: Platform: all
Project: ChimeraX

Description

According to pip/PEP docs, a requirement of 1.1 should be satisfied by 1.1.devblah-blah-blah, but it isn't. Need to get this fixed in pip.

Change History (18)

comment:1 by pett, 5 years ago

Milestone: 1.2

comment:2 by Tom Goddard, 5 years ago

Another case, not sure if the behavior is correct. A dependency on core ~=1.0rc1 is not satisfied by core 1.0. I encountered this with the RealSense toolshed package -- it gave me version 1.3 which required core >= 0.92 instead of 1.4 which required ~=1.0rc1.

comment:3 by Greg Couch, 5 years ago

Milestone: 1.21.3

Need to read PEP 440 very closely to construct argument. In code, the following assertion should not fail, but does:

#!/usr/bin/python3
from packaging.specifiers import Specifier
from packaging.version import Version

s = Specifier("~=1.0,>=1.2")
v = Version("1.2rc1")

assert s.contains(v, prereleases=True)

The scenario is that a package, A, is at version 1.0rc1, in anticipation of being released. And another package, B, depends on A and needs the 1.0 API, so it requires "A~=1.0". Package A is installed. And "pip install --pre B.whl" fails with

pip._internal.exceptions.DistributionNotFound: No matching distribution found for A~=1.0
Last edited 3 years ago by Greg Couch (previous) (diff)

comment:4 by Greg Couch, 4 years ago

Milestone: 1.31.4

We've worked around this by using regular version numbers for candidate releases. The production release of ChimeraX 1.2 was version 1.2.5. It would still be great if pip were improved.

comment:5 by Greg Couch, 3 years ago

Owner: changed from Greg Couch to Zach Pearson

comment:6 by Greg Couch, 3 years ago

Milestone: 1.41.5

comment:7 by Zach Pearson, 3 years ago

May be preempted. Using pip 22.0.4, packaging 21.3, and pyparsing 3.0.6:

>>> from packaging.specifiers import Specifier, SpecifierSet
>>> from packaging.version import Version
>>> s = Specifier('~=1.0,>=1.2')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/zjp/git/prof/rbvi/ChimeraX/.venv/lib/python3.9/site-packages/packaging/specifiers.py", line 98, in __init__
    raise InvalidSpecifier(f"Invalid specifier: '{spec}'")
packaging.specifiers.InvalidSpecifier: Invalid specifier: '~=1.0, >=1.2'
>>> s = SpecifierSet('~=1.0', '>=1.2')
>>> v = Version("1.2rc1")
>>> s.contains(v)
True
>>> s.contains(v, prereleases=False)
False
>>> s.contains(v, prereleases=True)
True

Curiously:

>>> s = SpecifierSet("~=1.0,>=1.2")
>>> s.contains(v)
False
>>> s.contains(v, prereleases=True)
False
>>> s.contains(v, prereleases=False)
False

So breaking them up seems to matter. That looks like the expected behavior to me; if you agree we can close this ticket.

comment:8 by pett, 3 years ago

So does our code need to be improved to use SpecifierSet somewhere? Or will this happen automatically in routines we call?

comment:9 by Zach Pearson, 3 years ago

I'm not sure.

If I try to install blastprotein, alphafold, and core into my virtual environment (with the previously mentioned versions of pip, etc), it complains about the version of core being 1.4.dev202205032347, but if I either add the --pre flag to pip install or just make blastprotein depend on 1.4dev then it works (other dependencies notwithstanding, that error just goes away).

comment:10 by Zach Pearson, 3 years ago

We never do import Specifier or SpecifierSet in our code, just Requirement.

Requirement will construct what I think is the buggy specifier set '~=1.0,>=1.2' instead of what works: '~=1.0', '>=1.2'

comment:11 by Zach Pearson, 3 years ago

>>> r = Requirement('tests == 1.4*')
>>> v = Version("1.4")
>>> v in r.specifier
False
>>> r = Requirement('tests == 1.4.*')
>>> v in r.specifier
True
>>> v = Version("1.4dev0")
>>> v in r.specifier
False
>>> r.specifier.contains(v, prereleases=True)
True

Semantic schmemantic, but I think depending on e.g. "chimerax.core == 1.4.*" which encompasses all pre-release or development versions and all minor bugfixes is the way to go. I think the ".*" should be implied after the "1.4", too, but it's not.

comment:12 by Greg Couch, 3 years ago

Specifier and SpecifierSet are using internally by pip. chimerax.core==1.4.* only matches 1.4.1, 1.4.2, etc. The use of Specifier and Requirement were to construct a simpler test case than trying to install a bundle than depends chimerax.core==1.4 into a candidate release where the version is 1.4rc####. Perhaps the test case needs fixing.

comment:13 by Zach Pearson, 3 years ago

>>> from packaging.requirements import Requirement
>>> from packaging.version import Version
>>> r = Requirement('tests == 1.4.*')
>>> r.specifier
<SpecifierSet('==1.4.*')>
>>> spec = r.specifier
>>> v = Version('1.4dev0')
>>> spec.contains(v)
False
>>> spec.contains(v, True)
True
>>> v = Version('1.4.9')
>>> spec.contains(v, True)
True
>>> spec.contains(v)
True
>>> v = Version('1.4rc1')
>>> spec.contains(v)
False
>>> spec.contains(v,True)
True

It looks like it also encompasses development and release candidate versions, but that could also be a bug.

comment:14 by Zach Pearson, 3 years ago

We might consider decoupling the naming of the core bundle version number from the ChimeraX release number. A Linux distribution will upgrade every package in a release at some point, but to me, the '1.4' version number is some guarantee of stability, representing frozen and tested code, and need not necessarily imply the core version number.

Last edited 3 years ago by Zach Pearson (previous) (diff)

comment:15 by Greg Couch, 3 years ago

Decoupling could be good. It would allow us to make major changes to the core API and not increase ChimeraX's major version number. Or not make any changes to the core (but I don't see that happening for a while). But that is a separate issue.

comment:16 by Zach Pearson, 3 years ago

I see in the compatible release version specifier section, they give the following example:

~= 2.2
>= 2.2, == 2.*

and state that the two clauses are equivalent: they both mean "any version over 2, and greater than or equal to 2.2".

>>> s_1 = SpecifierSet('~=2.2')
>>> s_2 = SpecifierSet('>=2.2,==2.*')

s_1 ends up containing 2.2 and 2.2.0 but not 2.2dev0 nor 2.2.0dev0. Same for s_2.

Tom's example from comment 2 also appears to work:

>>> s_3 = SpecifierSet('~=1.0rc1')
>>> s_3.contains('1.0')
True
>>> s_3.contains('1.0rc1')
True

Great , at least that bug is fixed. But we're still dealing with the reverse: allow a release candidate to satisfy a dependency on X.Y.

We can try to expand the ~= operator ourselves to see if that does it.

>>> s_4 = SpecifierSet('>=1.0,==1.*')
>>> s_4.contains('1.0')
True
>>> s_4.contains('1.0rc1')
False
>>> s_4.contains('1.0rc1', prereleases=True)
False

There's no more room to go truncating after that. 1.* is really 1[.*]*; It's the >= that's being the killjoy. pip install --pre does fail with ~=1.4 but succeeds with ==1.4. All I'm saying at this point is that I think that's an internally consistent workaround. I can see the argument going either way, it's highly semantic: "it's compatible release; 1.0dev0 is not >= 1.0" vs "it should be any version compatible, including pre-release".

But I think making better use of "==1.x" is serviceable. I only skimmed the docs, so I didn't see where the contradiction was.

comment:17 by Zach Pearson, 3 years ago

Critical typo: Fails with ~=1.4 and ==1.4 but succeeds with ==1.4.* (encompassing 1.4dev0 to 1.4.999999999rc999), so making better use of "==1.4.*" is serviceable.

comment:18 by Zach Pearson, 3 years ago

Milestone: 1.5
Note: See TracTickets for help on using tickets.