Update per comment on PR #629 . Sort in a version-aware manner

[kata198]

Update per comment on PR #629 . Sort in a version-aware manner such that 2.7.10 comes AFTER 2.7.2, not before.

This is in response to comment on PR #629 #629 (comment)

This changes the sort to a version-aware method, copied from my project "cmp_version" (https://github.com/kata198/cmp_version ). That project is Public Domain, so there are no licensing concerns.

Enjoy!

• Tim

[kata198]

Note this copies-and-pastes code from cmp_version instead of adding it as a dependency, I don't think we should add a dependency to setuptools. If you feel it belongs in another file ( I do, but I'm not sure where...), feel free ot modify accordingly.

[kata198]

I went ahead and moved cmp_version to the _vendor directory/module . Seems an apt location.

[jaraco]

I like the idea that you're keeping the _vendor code separate - that's a good plan.

Since cmp_version is a published package, it's probably best if we declare it as a vendored dependency - that is, in pkg_resources/_vendor/vendored.txt, declare the dependency for cmp_version, update pavement.py, and then run paver update_vendored (which is how all of the vendored dependencies are incorporated). Finally, instead of importing from pkg_resources._vendor, import from pkg_resources.extern. Doing this enables other package managers to treat cmp_version as a proper dependency and omit it from the vendored installation.

But more importantly - pkg_resources already relies on packaging, and I'd be really surprised if packaging doesn't already provide a suitable sort-order for these package version numbers.

>>> from pkg_resources.extern import packaging
>>> packaging.version
<module 'pkg_resources.extern.packaging.version' from '/Users/jaraco/Dropbox/code/main/setuptools/pkg_resources/_vendor/packaging/version.py'>
>>> low_ver = packaging.version.parse('2.7.2')
>>> high_ver = packaging.version.parse('2.7.10')
>>> low_ver < high_ver
True

Why not simply use that instead?

[kata198]

If you want, that's fine. I simplify included my cmp_version implementation
because I've been using that for a long time with python packages and I
know it works. The original implementation (without version-aware sorting)
adequately resolves the issue for me.

On Fri, Oct 28, 2016 at 1:41 PM, Jason R. Coombs notifications@github.com
wrote:

I like the idea that you're keeping the _vendor code separate - that's a
good plan.

Since cmp_version is a published package, it's probably best if we declare
it as a vendored dependency - that is, in pkg_resources/_vendor/
vendored.txt, declare the dependency for cmp_version, update pavement.py,
and then run paver update_vendored (which is how all of the vendored
dependencies are incorporated). Finally, instead of importing from
pkg_resources._vendor, import from pkg_resources.extern. Doing this
enables other package managers to treat cmp_version as a proper dependency
and omit it from the vendored installation.

But more importantly - pkg_resources already relies on packaging, and I'd
be really surprised if packaging doesn't already provide a suitable
sort-order for these package version numbers.

from pkg_resources.extern import packaging
packaging.version
<module 'pkg_resources.extern.packaging.version' from '/Users/jaraco/Dropbox/code/main/setuptools/pkg_resources/_vendor/packaging/version.py'>
low_ver = packaging.version.parse('2.7.2')
high_ver = packaging.version.parse('2.7.10')
low_ver < high_ver
True

Why not simply use that instead?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#829 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AIEbO7JQrf85Q2rQCkulMWW4-iZGWQbXks5q4jOtgaJpZM4KfnO8
.

[jaraco]

Okay. Thanks for this, but I think we're better off seeking a solution that doesn't involve bringing in more implementations of version parsing. Setuptools only recently deprecated its own version parsing logic for the packaging module. Appreciate the effort anyhow. I should be able to use your suggestion to devise another solution.

[jaraco]

Hmm. As I'm looking at it, I see that you're passing full filenames to the VersionString constructor, but VersionString and cmp_version only talk about taking version strings (i.e. "1.0.0b1" and not filenames like "mypkg-1.0.0b1.egg"). Perhaps it would have worked anyway because it was treating 'mypkg-1' as one part of the version. That makes me think it still might not handle the sorting of 'mypkg-10.0.0' and 'mypkg-2.0.0' properly.