Skip to content
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

Introduce ModuleAvailableCache #85

Closed
awaelchli opened this issue Jan 23, 2023 · 2 comments · Fixed by #86
Closed

Introduce ModuleAvailableCache #85

awaelchli opened this issue Jan 23, 2023 · 2 comments · Fixed by #86
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@awaelchli
Copy link
Contributor

awaelchli commented Jan 23, 2023

🚀 Feature

Motivation

RequirementCache exists, but it is very general. It works for both modules as well as packages. It has a very strict check for requirements of a package. This check can fail even if the package is actually importable successfully, see example: Lightning-AI/pytorch-lightning#16464

In these cases, the module_available check is better suited for what we want to check. But it can only be called as a function, not as a cache like the RequirementCache.

Pitch

Add ModuleAvailableCache, with a cache implementation like RequirementCache, but we will use the module_available function as as the check function.

Alternatives

User has to resolve any package conflicts in their environment, even if they are harmless.

Additional context

See Lightning-AI/pytorch-lightning#16464

@awaelchli awaelchli added enhancement New feature or request help wanted Extra attention is needed labels Jan 23, 2023
@awaelchli
Copy link
Contributor Author

cc @carmocca @Borda

@carmocca
Copy link
Contributor

Sounds good to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants