Conversation
make module-level optional deps checking get its own test with clear instructions about how to proceed and clear pointers to relevant line numbers, instead of bundling it with the function-level OptionalImportCheckResult, because module-level should never import optional deps (outside of "if TYPE_CHECKING" of course)
Collaborator
|
Thank you for these improvements! They look good, I did a few different sanity checks that things work well. While doing the checks I noticed that the error message from pytest was somewhat confusing when detecting module-level imports of optional dependencies. I added a commit to split the module-level import detection into its own functions and test, with a clearer message that module names and line numbers. I'm happy for this to merge into #1656 if it looks good to you! |
cmdupuis3
merged commit Sep 16, 2026
ba29789
into
sevans/_raise_hint_if_optional_deps_missing
17 checks passed
Sevans711
added a commit
that referenced
this pull request
Sep 17, 2026
…lled. (#1656) * add _raise_hint_if_optional_deps_missing * optional deps test ensure helpful hint gets raised * forgot pre-commit ruff formatting * fix opt deps error hint tests typos also add string matching to test_check_requires_viz_and_geo in both the installed_with_geo and the installed_with_viz cases, and left a comment about why it wasn't added in the installed_with_no_opts case. * add test_optional_deps files & ci commands * forgot pre-commit ruff formatting * fix ruff complaint about unused import * add healpix-sensitive optional deps test (maybe not necessary… but also trying to re-trigger CI jobs here, due to github actions downtime yesterday causing stalled jobs with no "rerun jobs" button available.) * fix optional deps test: cannot plot UxDataset * add _raise_hint_if_optional_deps_missing * optional deps test ensure helpful hint gets raised * forgot pre-commit ruff formatting * fix opt deps error hint tests typos also add string matching to test_check_requires_viz_and_geo in both the installed_with_geo and the installed_with_viz cases, and left a comment about why it wasn't added in the installed_with_no_opts case. * fix and test messages of missing opt deps hints * forgot pre-commit ruff formatting * test usage of _raise_hint_if_optional_deps_missing Ensures it is being used properly throughout the codebase. (Claude helped me write this but I read through it all and poked around slightly to improve function names and add more documentation.) * forgot pre-commit ruff formatting * fix: specify utf-8 encoding to avoid windows crash * improve installation docs page * Revert "improve installation docs page" This reverts commit 686d1f8. * More robust optional import linting for scoped imports (#1765) * More robust optional import linting for scoped imports * module-level optional deps gets its own test make module-level optional deps checking get its own test with clear instructions about how to proceed and clear pointers to relevant line numbers, instead of bundling it with the function-level OptionalImportCheckResult, because module-level should never import optional deps (outside of "if TYPE_CHECKING" of course) * (forgot pre-commit ruff formatting) --------- Co-authored-by: Sam Evans <47793072+Sevans711@users.noreply.github.com> * pytest monkeypatch when editing global dict --------- Co-authored-by: Christopher Dupuis <45972964+cmdupuis3@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
A lot of possible cases where optional import linting is needed are missed because it isn't fully walking the block scopes in the AST. These changes should help with that.