-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
replace package_available
with module_available
#16607
Conversation
⛈️ Required checks status: Has failure 🔴
Groups summary🔴 pytorch_lightning: Tests workflow
These checks are required after the changes to 🔴 pytorch_lightning: Azure GPU
These checks are required after the changes to 🔴 pytorch_lightning: Azure HPU
These checks are required after the changes to 🟢 pytorch_lightning: Azure IPU
These checks are required after the changes to 🔴 pytorch_lightning: Docs
These checks are required after the changes to 🔴 lightning_fabric: CPU workflow
These checks are required after the changes to 🟢 lightning_fabric: Azure GPU
These checks are required after the changes to 🔴 lightning_app: Tests workflow
These checks are required after the changes to 🟢 lightning_app: Examples
These checks are required after the changes to 🔴 lightning_app: Azure
These checks are required after the changes to 🟢 lightning_app: Docs
These checks are required after the changes to 🟢 mypy
These checks are required after the changes to 🟢 installThese checks are required after the changes to Thank you for your contribution! 💜
|
for more information, see https://pre-commit.ci
@Borda Have you checked why the spec is None for the lightning package? See the implementation: @lru_cache()
def package_available(package_name: str) -> bool:
"""Check if a package is available in your environment.
>>> package_available('os')
True
>>> package_available('bla')
False
"""
try:
return find_spec(package_name) is not None
except ModuleNotFoundError:
return False There must be something missing in the lightning package. |
yes, will check just want to patch it until we have it resolved :) |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #16607 +/- ##
=========================================
- Coverage 82% 64% -17%
=========================================
Files 435 416 -19
Lines 31517 31310 -207
=========================================
- Hits 25732 20124 -5608
- Misses 5785 11186 +5401 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are incorrect usages of the module_available utilities.
@@ -416,7 +416,7 @@ def run_app( | |||
) | |||
|
|||
|
|||
if RequirementCache("lightning-fabric>=1.9.0") or RequirementCache("lightning>=1.9.0"): | |||
if ModuleAvailableCache("lightning-fabric>=1.9.0") or ModuleAvailableCache("lightning>=1.9.0"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would this work? module available checks don't support version comparison!
@@ -34,7 +34,7 @@ | |||
from lightning.pytorch.utilities.model_summary import ModelSummary | |||
from lightning.pytorch.utilities.rank_zero import rank_zero_only | |||
|
|||
_NEPTUNE_AVAILABLE = RequirementCache("neptune-client") | |||
_NEPTUNE_AVAILABLE = ModuleAvailableCache("neptune-client") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not a module
@@ -691,7 +691,7 @@ def test_model_with_fsspec_as_parameter(tmpdir): | |||
trainer.test() | |||
|
|||
|
|||
@pytest.mark.skipif(RequirementCache("hydra-core<1.1"), reason="Requires Hydra's Compose API") | |||
@pytest.mark.skipif(ModuleAvailableCache("hydra-core<1.1"), reason="Requires Hydra's Compose API") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hydra-core is not a module
What does this PR do?
switch to rather use
module_available
until this is resolved: #16595 (comment)resolves #16810
Before submitting
PR review
Anyone in the community is welcome to review the PR.
Before you start reviewing, make sure you have read the review guidelines. In short, see the following bullet-list:
Did you have fun?
Make sure you had fun coding 🙃
cc @Borda @tchaton @carmocca @justusschock @awaelchli