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.
gh-120417: Add #noqa to used imports in the stdlib #120421
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
gh-120417: Add #noqa to used imports in the stdlib #120421
Changes from 6 commits
8347c30
19ceb4d
9b97f47
2189665
e31a9e2
7d93caa
13efcc9
648969c
0506006
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
test_collections
passes for me if I remove this import. Does it need to stay? It was added in 52f96d3There 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.
@erlend-aasland @kumaraditya303: Do you know/recall why this symbol is exposed?
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.
Oh,
test_deque
fails if the import is removed. I expecteddeque
to be tested as part oftest_collections
, but apparently it has its own test file. Could you add a comment that it is (apparently) required to expose it in thecollections
module in order for deque iterators to be pickled?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.
Sure, I added a comment.
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.
Which is the import being flagged as unused here? Why does it need to stay?
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.
pydoc.plain()
is an alias to_pyrepl.pager.plain()
. It has to stay since it's part of pydoc API, and test_pydoc fails if you remove it.# noqa
only applies to the current line, no?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.
I see, thanks. Maybe add a comment explaining that to the code?
It depends on the tool, the rule and the AST node. For example, in this:
the single
noqa
comment will cause Ruff to ignore the three unused importsdeque
,OrderedDict
andChainMap
, because they constitute a singleImportFrom
node in the AST and thenoqa
comment is applied on the starting line number of the AST node.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.
I moved the import to a separated statement and added a comment.