Update to current fastest env setitem #4556
Merged
Conversation
This file contains 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
Updated and re-ran benchmark tests for SubstitutionEnvironment.__setitem__. Added test for new (Python 3.0) string.isidentifier() method. That's actually exactly what we want, and it times out as the fastest method when combined with a membership test if the variable is already defined. Tweaked some comments about this performance consideration, and did other updates in bench/. Signed-off-by: Mats Wichmann <[email protected]>
Didn't notice that the changes for env.__setitem__ caused a quote mark to flip in the error output, failing one test (a back-quote went to a single regular quote). Updated the test, and snuck in the tools-performance-hack. Signed-off-by: Mats Wichmann <[email protected]>
bdbaddog
reviewed
Jun 27, 2024
bdbaddog
reviewed
Jun 27, 2024
bench/is_types.py
Outdated
| @@ -51,7 +53,7 @@ def cache_type_e_is_Dict(e): | |||
|
|
|||
| def cache_type_e_is_List(e): | |||
| t = type(e) | |||
| return t is list or isinstance(e, UserList) | |||
| return t is list or isinstance(e, UserList or isinstance(e, deque)) | |||
There was a problem hiding this comment.
Is this right? or inside the isinstance()?
There was a problem hiding this comment.
that got clumsy. I'll fix - think should be single isinstance
There was a problem hiding this comment.
yeah, it was supposed to look like the one a few lines later. pushed update.
Signed-off-by: Mats Wichmann <[email protected]>
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.
Updated and re-ran benchmark tests for
SubstitutionEnvironment.__setitem__. Added test for new (Python 3.0) string.isidentifier()method. That's actually exactly what we want, and it times out as the fastest method when combined with a membership test if the variable is already defined.Tweaked some comments about this performance consideration, and did other updates in
bench/.No doc impacts as there are no behavioral changes.
Contributor Checklist:
CHANGES.txt(and read theREADME.rst)