CI: linting check to ensure lib.NoDefault is only used for typing
#53901
Conversation
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
MarcoGorelli
added
Code Style
|
nice idea I think you're repeatedly walking up and down the same paths - do you want to check |
|
Improved from approx. 10s for all files to approx. 4s @MarcoGorelli Thanks for your suggestion! |
|
Well done! Would appreciate it if we could add a tiny little test case to scripts/tests/test_validate_unwanted_patterns.py too Other than that, looks good! |
|
Didn't notice that there are tests for linting scripts, thanks for the reminder @MarcoGorelli. Added some simple tests now. |
MarcoGorelli
approved these changes
Jun 28, 2023
mroeschke
reviewed
Jun 28, 2023
mroeschke
approved these changes
Jun 29, 2023
|
Nice thanks @Charlie-XIAO |
Daquisu
pushed a commit
to Daquisu/pandas
that referenced
this pull request
Jul 8, 2023
…andas-dev#53901) * CI add liniting to check NoDefault used only for typing * minor modification * reduce cost * tests added for the new linting check * types_or -> types because only python * rephrase pre-commit hook name * rephrase more * fix failing tests: * retrigger checks * retrigger checks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
lib.no_defaultinstead oflib.NoDefaultin.pivot#53877 (comment)In this PR, I added a pre-commit hook to check that
pandas._libs.lib.NoDefaultis used only for typing. This is invoked by #53877. Before that PR, this new hook correctly catches all misusedNoDefault:After that PR, the check passes normally:
In fact I don't work with ASTs very often so not sure if this check covers all possibilties. Please let me know if any modification is needed. (The runtime is a bit long but seems acceptable, approximately 10s to check all files.)