Skip to content

Conversation

@ericspod
Copy link
Member

@ericspod ericspod commented Jun 29, 2021

Signed-off-by: Eric Kerfoot eric.kerfoot@kcl.ac.uk

Fixes #2479 .

Description

This fixes an issue with version strings and replaces some functionality reliant on an optional package with an internal solution. Version strings should contain integers separated by ".", any component not containing an integer is broken into an initial integer component and the rest. The integers from this are compared when comparing versions, so "0.6.0rc0" will be treated the same as "0.6.0". Git version suffixes are ignored.

Status

Ready

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

ericspod added 2 commits June 29, 2021 16:35
Signed-off-by: Eric Kerfoot <eric.kerfoot@kcl.ac.uk>
Signed-off-by: Eric Kerfoot <eric.kerfoot@kcl.ac.uk>
@ericspod ericspod requested review from Nic-Ma and wyli June 29, 2021 15:58
ericspod added 4 commits June 29, 2021 16:59
Signed-off-by: Eric Kerfoot <eric.kerfoot@kcl.ac.uk>
Signed-off-by: Eric Kerfoot <eric.kerfoot@kcl.ac.uk>
Signed-off-by: Eric Kerfoot <eric.kerfoot@kcl.ac.uk>
Signed-off-by: Eric Kerfoot <eric.kerfoot@kcl.ac.uk>
def _try_cast(val):
val = val.strip()
try:
m = re.match("(\\d+)(.*)", val)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should prefer optional_import("pkg_resources", name="parse_version") if it is installed... this version_leq will be used in many cases, better to make it as robust as possible

Copy link
Contributor

@wyli wyli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! if you don't have time to add optional_import("pkg_resources", name="parse_version"), I'll do it later today

Signed-off-by: Eric Kerfoot <eric.kerfoot@kcl.ac.uk>
@ericspod ericspod enabled auto-merge (squash) June 29, 2021 17:11
@ericspod ericspod merged commit c6efca7 into Project-MONAI:dev Jun 29, 2021
@ericspod
Copy link
Member Author

This should sort the optional dependency on pkg_resources now

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

enhance the version comparison utililty

2 participants