Skip to content

Commit 960c59b

Browse files
authored
Fix packaging imports in version comparison logic (#8347)
Fixes #8349 ### Description The current behaviour is that `pkging, has_ver = optional_import("packaging.Version")` always returns `has_ver=False` because the import always fails (the `Version` class is exposed by the `packaging.version` submodule). This issue previously didn't surface, because when the import fails, it would just continue to use the fallback logic. However, there seem to be more hidden and more severe implications, which ultimately led me to discovering this particular bug: Function like `floor_divide()` in `monai.transforms` that check the module version using this logic are called many times in common ML dataloading workflows. The failed imports somehow can lead to OOM errors and the main process being killed (see #8348). Maybe when `optional_import` fails to import a module, the lazy exceptions somehow stack up in memory when this function is called many times in a short time period? ### Types of changes <!--- Put an `x` in all the boxes that apply, and remove the not applicable items --> - [x] 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 --disttests`. - [ ] In-line docstrings updated. - [ ] Documentation updated, tested `make html` command in the `docs/` folder.
1 parent 0a85eed commit 960c59b

File tree

1 file changed

+6
-6
lines changed

1 file changed

+6
-6
lines changed

monai/utils/module.py

+6-6
Original file line numberDiff line numberDiff line change
@@ -540,11 +540,11 @@ def version_leq(lhs: str, rhs: str) -> bool:
540540
"""
541541

542542
lhs, rhs = str(lhs), str(rhs)
543-
pkging, has_ver = optional_import("packaging.Version")
543+
pkging, has_ver = optional_import("packaging.version")
544544
if has_ver:
545545
try:
546-
return cast(bool, pkging.version.Version(lhs) <= pkging.version.Version(rhs))
547-
except pkging.version.InvalidVersion:
546+
return cast(bool, pkging.Version(lhs) <= pkging.Version(rhs))
547+
except pkging.InvalidVersion:
548548
return True
549549

550550
lhs_, rhs_ = parse_version_strs(lhs, rhs)
@@ -567,12 +567,12 @@ def version_geq(lhs: str, rhs: str) -> bool:
567567
568568
"""
569569
lhs, rhs = str(lhs), str(rhs)
570-
pkging, has_ver = optional_import("packaging.Version")
570+
pkging, has_ver = optional_import("packaging.version")
571571

572572
if has_ver:
573573
try:
574-
return cast(bool, pkging.version.Version(lhs) >= pkging.version.Version(rhs))
575-
except pkging.version.InvalidVersion:
574+
return cast(bool, pkging.Version(lhs) >= pkging.Version(rhs))
575+
except pkging.InvalidVersion:
576576
return True
577577

578578
lhs_, rhs_ = parse_version_strs(lhs, rhs)

0 commit comments

Comments
 (0)