fix-_is_package_available-unify-behavior-for-available-failing-import #31798
+7
−0
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Discussion is in #31793. Turns out it does not completely fix it, but allows a unified handling of libraries that are installed, hence
importlib.metadata.version
can read their version, but fail to import with ImportError. Current behavior for these packages with_is_package_available()
is to returnTrue
. While this is not coherent with howtorch
is treated in the same function:and also does not align with the name of the function.
I think it would be beneficial to merge this PR, because the behavior of the function is a little counterintuitive and results in problems down the line when the developer is not fully aware of how it treats these corner cases.
#31793 could be fixed with this + skipping the import guard check in
dynamic_module_utils.py
, line 181 -- model works the same with and without flash_attn. But I think this is up to the model developers how to handle it.Finally, I am not sure of the logging level there, but I guess some information to the user would also be beneficial.
Fixes :
Initially:
With PR:
Before submitting
Pull Request section?
documentation guidelines, and
here are tips on formatting docstrings.
No, don't think it's needed. It is aligned with function signature.
Who can review?
@LysandreJik @amyeroberts @muellerzr