[22.01] Workaround to numeric sorting in the local portion of tool versions if they are galaxy "build" numbers

[natefoo]

On usegalaxy.org, we have installed a few different "+galaxyN" versions of Circos 0.69.8, including +galaxy1, +galaxy7, and +galaxy10. The version that shows up in the tool panel is +galaxy7, although +galaxy10 is selectable from the tool form version dropdown. This is because of the PEP-440 rules regarding sorting of the local version identifier.

This change causes local version identifiers of the format ^\+galaxy(.+)$ to be converted to +galaxy.(\1) so that if the stuff after +galaxy is an integer, it will sort numerically instead of lexicographically.

WIP/Draft because @nsoranzo kindly offered to take a look at the proper handling of tool lineages.

How to test the changes?

(Select all options that apply)

• I've included appropriate automated tests.
• This is a refactoring of components with existing test coverage.
• Instructions for manual testing are as follows:
  1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

• I agree to license these contributions under Galaxy's current license.
• I agree to allow the Galaxy committers to license these and all my past contributions to the core galaxy codebase under the MIT license. If this condition is an issue, uncheck and just let us know why with an e-mail to [email protected].

[jdavcs]

Would you mind adding a simple unit test? Just to verify that whatever is passed to return packaging.version.parse(version) is correct?

[jdavcs]

Would you mind adding a simple unit test? Just to verify that whatever is passed to return packaging.version.parse(version) is correct?

As a follow-up to the discussion at the pre-release meeting, I don't think we need to initialize a tool here for testing. The only thing that needs testing here is the logic modifying the version string. One simple way to do it would be to refactor the version_object property extracting a helper method that takes a string and returns a, possibly, modified string. Thus, the property would call the helper and then, call packaging.version.parse(version). Then we don't need to mock out self and packaging and just test the helper with a dirt simple unit test.

[nsoranzo]

LegacyVersion has just been removed from packaging in pypa/packaging#407, meaning that packaging.version.parse() will raise an exception for version strings that are not PEP 440 compliant. As mentioned, I think we need to vendorize the latest version and add our changes, so it can be reused also for tool lineages in lib/galaxy/tool_util/toolbox/lineages/interface.py

[pradyunsg]

FWIW, you can do packaging.version.Version today. That PR changes parse to basically literally be:

def parse(string):
    return Version(string)

(with a diff variable name)

[nsoranzo]

@pradyunsg Thanks for the tip, I'm aware, but unfortunately this doesn't help for us because we have to support user-defined tool versions which are often not PEP 440 compliant (e.g. "4.3.1t") to not break reproducibility of old pipelines.

[pradyunsg]

Could you drop a comment on pypa/packaging#502?

Multiple folks have expressed interest in having the LegacyVersion logic maintained somewhere, for use cases other than Python's packaging tooling. A linient-version package makes a lot of sense for such use cases -- I don't have any stake in those usecases personally, but it might make sense for someone who does to pull the code out for reuse. That'd make life easier for other users too.

Even if the final maintainer of such a package isn't you, having your interest expressed on that issue would still be useful. :)