feature request: Version comparison with str ?

[neutrinoceros]

Would it make sense to add support for direct comparison between Version objects and str ?
Currently when using packaging.version.Version to write 'versioned code branches' to support multiple versions of a third party package (e.g. foo), one need to import the Version class everywhere versions are being compared, which means that each module needs to import and parse the version for foo, or import it from a common module (say a.py), and import Version.

# a.py
import foo
from packaging.version import Version

FOO_VERSION = Version(foo.__version__)

# b.py
from .a import FOO_VERSION
from packaging.version import Version

if FOO_VERSION >= Version("2.0"):
    ...
else:
    ...

Now say the Version class knows how to compare with str directly, the b.py module could be simplified as

# b.py
from .a import FOO_VERSION

if FOO_VERSION >= "2.0":
    ...
else:
    ...

now multiply this by dozens of modules and more branches, and this noticeably alleviates the maintenance cost of these branches.

I would like to offer such a contribution, but is this something people other than me are interested in ?

[brettcannon]

Since the string would have to be coerced to a Version instance, I would rather lean on "explicit is better than implicit" and not support this.

[brettcannon]

And you could add an attribute on foo which stores the parsed version if you're concerned about performance. You could also cache by key somewhere the version string and get back the Version instance.

[neutrinoceros]

Performance isn't an issue AFAIK. Thank you for your response. Feel free to close this :)

[uranusjr]

#454 would kind of help with this as well since you'd always call Version instead and at least don't need to remember the type of FOO_VERSION.

[hmaarrfk]

Would you consider, as an exception to your rule, to allow comparisons to strings, but to emit a warning? This would greatly help in the transition away from distutils.

I've been hit by a few bugs that we couldn't catch in our CI procedure migrating away from distutils.

[brettcannon]

@hmaarrfk can you share the code? I don't quite understand how this would help with a distutils migration.

[hmaarrfk]

In [1]: from distutils.version import LooseVersion as Version

In [2]: Version('1.2.0') > '1.1.19'
<ipython-input-2-a0e8c8992dad>:1: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead.
  Version('1.2.0') > '1.1.19'
True

In [3]: from packaging.version import Version

In [4]: Version('1.2.0') > '1.1.19'
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-4-a0e8c8992dad> in <module>
----> 1 Version('1.2.0') > '1.1.19'

TypeError: '>' not supported between instances of 'Version' and 'str'

[hmaarrfk]

In this example, it is obvious that '1.1.19' is a string. However, in python, it is often hard to inspect things in detail.

Therefore, I would want this TypeError to issue a warning instead, guiding users through the migration path.

Typically you achieve this with:

warn(..., stacklevel=2)

pointing to the user's line of code that should be changed.

[pradyunsg]

to issue a warning instead,

And how should it behave in terms of the resturn value? If it is still erroring out, then presenting that warning isn't particularly useful.

And if the new behaviour involves coercing the string (or no longer erroring out), then we're (a) introducing a backwards-incompatible behaviour change and (b) see the rationale provided above for not doing this.

If you absolutely want things to behave like distutils' LooseVersion, feel free to create a subclass of packaging.version.Version that overrides the comparision operators to behave how you want, and using that subclass instead. I don't see any way to provide that functionality within the constraints we're operating in.

[hmaarrfk]

introducing a backwards-incompatible behaviour change

I'm not sure where backward incompatibility was discussed. That said, I understand the importance of that.

If you absolutely want things to behave like distutils' LooseVersion, feel free to create a subclass of packaging.version.Version that overrides the comparision operators to behave how you want, and using that subclass instead. I don't see any way to provide that functionality within the constraints we're operating in.

This is a very fair point, and definitely makes things easier from packaging's point of view. I understand (though, I don't entirely agree with) your decision to take this approach.

My main objection to not allowing comparisons to string is that code that depends on version comparison might be locating in a compatibility shim. This makes it extremely hard to test on CIs where quite often, strange package combinations don't exist. Often developers test against:
* The lowest dependencies allowed.
* The latest available versions of all packages.
* Nothing in between.
This means that the only time you actually test for these kinds of comparisons is in the wild.

The recommendation to use packaging.version.Version to replace distutil's LooseVersion is thus, in my opinion, not very maintainer friendly.

[brettcannon]

The recommendation to use packaging.version.Version to replace distutil's LooseVersion is thus, in my opinion, not very maintainer friendly.

I'm not sure where you're getting that warning from (maybe setuptools since that isn't in Python's stdlib itself), but it isn't coming from us and this project.