From e82db5faf7e0cac3def8f3a9633835e34f4dd0db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mochol=C3=AD?= Date: Tue, 21 Feb 2023 17:52:54 +0100 Subject: [PATCH 1/8] Fixing tests --- src/lightning_utilities/core/imports.py | 77 +++++++++---------------- tests/unittests/core/test_imports.py | 14 ++--- 2 files changed, 30 insertions(+), 61 deletions(-) diff --git a/src/lightning_utilities/core/imports.py b/src/lightning_utilities/core/imports.py index 6d93fe23..2f170790 100644 --- a/src/lightning_utilities/core/imports.py +++ b/src/lightning_utilities/core/imports.py @@ -6,6 +6,7 @@ import importlib import warnings from functools import lru_cache +from importlib import metadata from importlib.util import find_spec from types import ModuleType from typing import Any, Callable, List, Optional @@ -14,12 +15,6 @@ from packaging.requirements import Requirement from packaging.version import Version -try: - from importlib import metadata -except ImportError: - # Python < 3.8 - import importlib_metadata as metadata # type: ignore - @lru_cache() def package_available(package_name: str) -> bool: @@ -84,7 +79,11 @@ def compare_version(package: str, op: Callable, version: str, use_base_version: class RequirementCache: - """Boolean-like class for check of requirement with extras and version specifiers. + """Boolean-like class to check for requirement and module availability. + + Args: + requirement: The requirement to check, version specifiers are allowed. + module: The optional module to try to import if the requirement check fails. >>> RequirementCache("torch>=0.1") Requirement 'torch>=0.1' met @@ -92,59 +91,35 @@ class RequirementCache: True >>> bool(RequirementCache("torch>100.0")) False - """ - - def __init__(self, requirement: str) -> None: - self.requirement = requirement - - def _check_requirement(self) -> None: - if not hasattr(self, "available"): - try: - pkg_resources.require(self.requirement) - self.available = True - self.message = f"Requirement {self.requirement!r} met" - except Exception as ex: - self.available = False - self.message = f"{ex.__class__.__name__}: {ex}. HINT: Try running `pip install -U {self.requirement!r}`" - - def __bool__(self) -> bool: - """Format as bool.""" - self._check_requirement() - return self.available - - def __str__(self) -> str: - """Format as string.""" - self._check_requirement() - return self.message - - def __repr__(self) -> str: - """Format as string.""" - return self.__str__() - - -class ModuleAvailableCache: - """Boolean-like class for check of module availability. - - >>> ModuleAvailableCache("torch") - Module 'torch' available - >>> bool(ModuleAvailableCache("torch")) + >>> RequirementCache("torch") + Requirement 'torch' met + >>> bool(RequirementCache("torch")) True - >>> bool(ModuleAvailableCache("unknown_package")) + >>> bool(RequirementCache("unknown_package")) False """ - def __init__(self, module: str) -> None: + def __init__(self, requirement: str, module: Optional[str] = None) -> None: + self.requirement = requirement self.module = module def _check_requirement(self) -> None: if hasattr(self, "available"): return - - self.available = module_available(self.module) - if self.available: - self.message = f"Module {self.module!r} available" - else: - self.message = f"Module not found: {self.module!r}. HINT: Try running `pip install -U {self.module}`" + try: + # first try the pkg_resources requirement + pkg_resources.require(self.requirement) + self.available = True + self.message = f"Requirement {self.requirement!r} met" + except Exception as ex: + # assume they match if module is not passed + module = self.requirement if self.module is None else self.module + # if it fails, try to import it: sometimes `pkg_resources.require()` fails but the module is importable + self.available = module_available(module) + if self.available: + self.message = f"Module {module!r} available" + else: + self.message = f"{ex.__class__.__name__}: {ex}. HINT: Try running `pip install -U {self.requirement!r}`" def __bool__(self) -> bool: """Format as bool.""" diff --git a/tests/unittests/core/test_imports.py b/tests/unittests/core/test_imports.py index 161077f7..e517719b 100644 --- a/tests/unittests/core/test_imports.py +++ b/tests/unittests/core/test_imports.py @@ -1,5 +1,6 @@ import operator import re +from importlib.metadata import PackageNotFoundError import pytest @@ -8,17 +9,10 @@ get_dependency_min_version_spec, lazy_import, module_available, - ModuleAvailableCache, RequirementCache, requires, ) -try: - from importlib.metadata import PackageNotFoundError -except ImportError: - # Python < 3.8 - from importlib_metadata import PackageNotFoundError - def test_module_exists(): assert module_available("_pytest") @@ -54,9 +48,9 @@ def test_requirement_cache(): def test_module_available_cache(): - assert ModuleAvailableCache("pytest") - assert not ModuleAvailableCache("this_module_is_not_installed") - assert "pip install -U this_module_is_not_installed" in str(ModuleAvailableCache("this_module_is_not_installed")) + assert RequirementCache("pytest") + assert not RequirementCache("this_module_is_not_installed") + assert "pip install -U 'this_module_is_not_installed" in str(RequirementCache("this_module_is_not_installed")) def test_get_dependency_min_version_spec(): From 42e38ed323840c97edb440886ab2a52bdb70ff0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mochol=C3=AD?= Date: Tue, 21 Feb 2023 18:19:10 +0100 Subject: [PATCH 2/8] Refactor --- src/lightning_utilities/core/imports.py | 17 +++++++++-------- tests/unittests/core/test_imports.py | 13 +++++++++---- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/lightning_utilities/core/imports.py b/src/lightning_utilities/core/imports.py index 2f170790..78201afc 100644 --- a/src/lightning_utilities/core/imports.py +++ b/src/lightning_utilities/core/imports.py @@ -112,14 +112,15 @@ def _check_requirement(self) -> None: self.available = True self.message = f"Requirement {self.requirement!r} met" except Exception as ex: - # assume they match if module is not passed - module = self.requirement if self.module is None else self.module - # if it fails, try to import it: sometimes `pkg_resources.require()` fails but the module is importable - self.available = module_available(module) - if self.available: - self.message = f"Module {module!r} available" - else: - self.message = f"{ex.__class__.__name__}: {ex}. HINT: Try running `pip install -U {self.requirement!r}`" + self.available = False + self.message = f"{ex.__class__.__name__}: {ex}. HINT: Try running `pip install -U {self.requirement!r}`" + requirement_contains_version_specifier = "~=<>" in self.requirement + if not requirement_contains_version_specifier or self.module is not None: + module = self.requirement if self.module is None else self.module + # sometimes `pkg_resources.require()` fails but the module is importable + self.available = module_available(module) + if self.available: + self.message = f"Module {module!r} available" def __bool__(self) -> bool: """Format as bool.""" diff --git a/tests/unittests/core/test_imports.py b/tests/unittests/core/test_imports.py index e517719b..85103333 100644 --- a/tests/unittests/core/test_imports.py +++ b/tests/unittests/core/test_imports.py @@ -46,11 +46,16 @@ def test_requirement_cache(): assert not RequirementCache(f"pytest<{pytest.__version__}") assert "pip install -U '-'" in str(RequirementCache("-")) + # invalid requirement is skipped by valid module + assert RequirementCache(f"pytest<{pytest.__version__}", "pytest") -def test_module_available_cache(): - assert RequirementCache("pytest") - assert not RequirementCache("this_module_is_not_installed") - assert "pip install -U 'this_module_is_not_installed" in str(RequirementCache("this_module_is_not_installed")) + cache = RequirementCache("this_module_is_not_installed") + assert not cache + assert "pip install -U 'this_module_is_not_installed" in str(cache) + + cache = RequirementCache("this_module_is_not_installed", "this_also_is_not") + assert not cache + assert "pip install -U 'this_module_is_not_installed" in str(cache) def test_get_dependency_min_version_spec(): From 300684382af76943aa0d1a68cb4761e82c4f79e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mochol=C3=AD?= Date: Tue, 21 Feb 2023 18:20:56 +0100 Subject: [PATCH 3/8] Fix --- src/lightning_utilities/core/imports.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lightning_utilities/core/imports.py b/src/lightning_utilities/core/imports.py index 78201afc..1e52d737 100644 --- a/src/lightning_utilities/core/imports.py +++ b/src/lightning_utilities/core/imports.py @@ -114,7 +114,7 @@ def _check_requirement(self) -> None: except Exception as ex: self.available = False self.message = f"{ex.__class__.__name__}: {ex}. HINT: Try running `pip install -U {self.requirement!r}`" - requirement_contains_version_specifier = "~=<>" in self.requirement + requirement_contains_version_specifier = any(c in self.requirement for c in "~=<>") if not requirement_contains_version_specifier or self.module is not None: module = self.requirement if self.module is None else self.module # sometimes `pkg_resources.require()` fails but the module is importable From f3f3f9e77ffd6f092dc1085008ae48767f5a15fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mochol=C3=AD?= Date: Tue, 21 Feb 2023 18:21:58 +0100 Subject: [PATCH 4/8] Py3.8 compat --- src/lightning_utilities/core/imports.py | 7 ++++++- tests/unittests/core/test_imports.py | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/lightning_utilities/core/imports.py b/src/lightning_utilities/core/imports.py index 1e52d737..dcf5469a 100644 --- a/src/lightning_utilities/core/imports.py +++ b/src/lightning_utilities/core/imports.py @@ -6,7 +6,6 @@ import importlib import warnings from functools import lru_cache -from importlib import metadata from importlib.util import find_spec from types import ModuleType from typing import Any, Callable, List, Optional @@ -15,6 +14,12 @@ from packaging.requirements import Requirement from packaging.version import Version +try: + from importlib import metadata +except ImportError: + # Python < 3.8 + import importlib_metadata as metadata # type: ignore + @lru_cache() def package_available(package_name: str) -> bool: diff --git a/tests/unittests/core/test_imports.py b/tests/unittests/core/test_imports.py index 85103333..66caece5 100644 --- a/tests/unittests/core/test_imports.py +++ b/tests/unittests/core/test_imports.py @@ -1,6 +1,5 @@ import operator import re -from importlib.metadata import PackageNotFoundError import pytest @@ -13,6 +12,12 @@ requires, ) +try: + from importlib.metadata import PackageNotFoundError +except ImportError: + # Python < 3.8 + from importlib_metadata import PackageNotFoundError + def test_module_exists(): assert module_available("_pytest") From 45e81f213aebd058c61689abdb99f33ec0ec7e90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mochol=C3=AD?= Date: Tue, 21 Feb 2023 18:30:19 +0100 Subject: [PATCH 5/8] Simpler --- src/lightning_utilities/core/imports.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lightning_utilities/core/imports.py b/src/lightning_utilities/core/imports.py index dcf5469a..dee4f2b2 100644 --- a/src/lightning_utilities/core/imports.py +++ b/src/lightning_utilities/core/imports.py @@ -119,7 +119,7 @@ def _check_requirement(self) -> None: except Exception as ex: self.available = False self.message = f"{ex.__class__.__name__}: {ex}. HINT: Try running `pip install -U {self.requirement!r}`" - requirement_contains_version_specifier = any(c in self.requirement for c in "~=<>") + requirement_contains_version_specifier = any(c in self.requirement for c in "=<>") if not requirement_contains_version_specifier or self.module is not None: module = self.requirement if self.module is None else self.module # sometimes `pkg_resources.require()` fails but the module is importable From 807402d12604af8b595790f26270674cc47a2294 Mon Sep 17 00:00:00 2001 From: Jirka Date: Wed, 22 Feb 2023 12:36:48 +0100 Subject: [PATCH 6/8] revert ModuleAvailableCache --- src/lightning_utilities/core/imports.py | 39 +++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/lightning_utilities/core/imports.py b/src/lightning_utilities/core/imports.py index dee4f2b2..38456c4d 100644 --- a/src/lightning_utilities/core/imports.py +++ b/src/lightning_utilities/core/imports.py @@ -142,6 +142,45 @@ def __repr__(self) -> str: return self.__str__() +class ModuleAvailableCache: + """Boolean-like class for check of module availability. + + >>> ModuleAvailableCache("torch") + Module 'torch' available + >>> bool(ModuleAvailableCache("torch")) + True + >>> bool(ModuleAvailableCache("unknown_package")) + False + """ + + def __init__(self, module: str) -> None: + self.module = module + + def _check_requirement(self) -> None: + if hasattr(self, "available"): + return + + self.available = module_available(self.module) + if self.available: + self.message = f"Module {self.module!r} available" + else: + self.message = f"Module not found: {self.module!r}. HINT: Try running `pip install -U {self.module}`" + + def __bool__(self) -> bool: + """Format as bool.""" + self._check_requirement() + return self.available + + def __str__(self) -> str: + """Format as string.""" + self._check_requirement() + return self.message + + def __repr__(self) -> str: + """Format as string.""" + return self.__str__() + + def get_dependency_min_version_spec(package_name: str, dependency_name: str) -> str: """Return the minimum version specifier of a dependency of a package. From 3139b27dac74b24178484c21a9f4fb4aa8092252 Mon Sep 17 00:00:00 2001 From: Jirka Date: Wed, 22 Feb 2023 12:43:50 +0100 Subject: [PATCH 7/8] tests --- tests/unittests/core/test_imports.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/unittests/core/test_imports.py b/tests/unittests/core/test_imports.py index 66caece5..059fb9aa 100644 --- a/tests/unittests/core/test_imports.py +++ b/tests/unittests/core/test_imports.py @@ -8,6 +8,7 @@ get_dependency_min_version_spec, lazy_import, module_available, + ModuleAvailableCache, RequirementCache, requires, ) @@ -63,6 +64,12 @@ def test_requirement_cache(): assert "pip install -U 'this_module_is_not_installed" in str(cache) +def test_module_available_cache(): + assert ModuleAvailableCache("pytest") + assert not ModuleAvailableCache("this_module_is_not_installed") + assert "pip install -U this_module_is_not_installed" in str(ModuleAvailableCache("this_module_is_not_installed")) + + def test_get_dependency_min_version_spec(): attrs_min_version_spec = get_dependency_min_version_spec("pytest", "attrs") assert re.match(r"^>=[\d.]+$", attrs_min_version_spec) From 2c7d8c2d8b462cda5693234d69e34c530d4e9e34 Mon Sep 17 00:00:00 2001 From: Jirka Date: Wed, 22 Feb 2023 12:46:32 +0100 Subject: [PATCH 8/8] chlog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a455e22a..2926d200 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- +- More resilient RequirementCache that checks for module import-ability ([#112](https://github.com/Lightning-AI/utilities/pull/112)) ## [0.7.0] - 2023-02-20