Conversation
Avasam
commented
Jul 20, 2024
| ] | ||
| new_commands = [ | ||
| # Type the same as distutils.command.install.install.sub_commands | ||
| new_commands: ClassVar[list[tuple[str, Callable[[Any], bool] | None]]] = [ |
Contributor
Author
There was a problem hiding this comment.
This was causing invariance issues when trying to append install.new_commands to install.sub_commands at the end of this module
Avasam
commented
Jul 20, 2024
| 'params', | ||
| WHEEL_INSTALL_TESTS, | ||
| ids=list(params['id'] for params in WHEEL_INSTALL_TESTS), | ||
| ids=[params['id'] for params in WHEEL_INSTALL_TESTS], |
Contributor
Author
There was a problem hiding this comment.
Not a necessary change, but I noticed this improvement whilst I was looking at this line. After realizing the best fix was an annotation change, I left this line as a comprehension. Would've been caught by https://docs.astral.sh/ruff/rules/unnecessary-generator-list/
I can revert this line if you'd prefer no runtime change
Member
There was a problem hiding this comment.
I'm good with it. Thanks for highlighting!
85e6c42 to
3aba4d4
Compare
jaraco
approved these changes
Jul 21, 2024
| 'params', | ||
| WHEEL_INSTALL_TESTS, | ||
| ids=list(params['id'] for params in WHEEL_INSTALL_TESTS), | ||
| ids=[params['id'] for params in WHEEL_INSTALL_TESTS], |
Member
There was a problem hiding this comment.
I'm good with it. Thanks for highlighting!
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
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.
Summary of changes
follow_imports = silentcomment to be more generalized / future-prooftypes-setuptoolsfrom typeshed. Right now a CI failure is expected almost every version, especially as more annotations are added to setuptools, and fixes upstreamed to typeshed. Which would require a lock, then unlock PRs everytime (more churn than just locking and updating when ready), or non-obvious fixes (mainly due to distutils subtyping). Oncetypes-setuptoolsis obsoleted, things should be more stable. @jaracoGoes towards unblocking #4352
Closes #4432
Pull Request Checklist
newsfragments/. (no user facing changes)(See documentation for details)