{{ message }}
Speed up stubs suggestions#17965
Merged
Merged
Conversation
This is starting to show up on profiles, especially incremental ones
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
JukkaL
approved these changes
Oct 16, 2024
JukkaL
left a comment
JukkaL
left a comment
Collaborator
There was a problem hiding this comment.
Thanks! Looks good, left some minor optional comments.
Collaborator
There was a problem hiding this comment.
I wonder if module.split(".", 1)[0] would be faster.
| assert stub_distribution_name("babel") == "types-babel" | ||
| assert stub_distribution_name("google.cloud.ndb") == "types-google-cloud-ndb" | ||
| assert stub_distribution_name("google.cloud.ndb.submodule") == "types-google-cloud-ndb" | ||
| assert stub_distribution_name("google.cloud.unknown") is None |
Collaborator
There was a problem hiding this comment.
Maybe also test good.protobuf, since it's a slightly different case as there are two packages under the google prefix?
| def approved_stub_package_exists(prefix: str) -> bool: | ||
| return is_legacy_bundled_package(prefix) or prefix in non_bundled_packages | ||
| def approved_stub_package_exists(module: str) -> bool: | ||
| components = module.split(".") |
Collaborator
There was a problem hiding this comment.
Again, it might be slightly faster to use module.split(".", 1)[0] to calculate prefix, and only do the full split in the body of the last if statement as needed.
This comment has been minimized.
This comment has been minimized.
Contributor
hauntsaninja
added a commit
that referenced
this pull request
Oct 20, 2024
See #17948 This is starting to show up on profiles - 1.01x faster on clean (below noise) - 1.02x faster on long - 1.02x faster on openai - 1.01x faster on openai incremental I had a dumb bug that was preventing the optimisation for a while, I'll see if I can make it even faster. Currently it's a small improvement We could also get rid of the legacy stuff in mypy 2.0
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.
See #17948
This is starting to show up on profiles
I had a dumb bug that was preventing the optimisation for a while, I'll see if I can make it even faster. Currently it's a small improvement
We could also get rid of the legacy stuff in mypy 2.0