Adds cache option to remap and use in research#394
Merged
mahmoud merged 1 commit intomahmoud:masterfrom Jan 28, 2026
Merged
Conversation
There is a mismatch between the caching of transformed objects in the
`remap` function and the need for `research` to traverse all sub
trees. For most values this is inconsequential (like atomic ints,
etc.) but for small tuples (e.g. `("hello",)`) these get compiled as
the same value and return the same `id(...)`. In `remap` these get
cached and never get `enter` called on them and thus the hooks for
`research` to return the values never gets called.
In this fix an option to disable/enable the cache is introduced to the
`remap` function which simply disables using transformed values from
the cache. Then in the `research` function caching is turned off.
The existing `remap` behavior is maintained as caching by default is
turned on.
fixes: mahmoud#393
Owner
|
Ohhh, that's a good find, and well-explicated, too. Appreciate the test and keeping the PR minimal. |
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.
There is a mismatch between the caching of transformed objects in the
remapfunction and the need forresearchto traverse all sub trees. For most values this is inconsequential (like atomic ints, etc.) but for small tuples (e.g.("hello",)) these get compiled as the same value and return the sameid(...). Inremapthese get cached and never getentercalled on them and thus the hooks forresearchto return the values never gets called.In this fix an option to disable/enable the cache is introduced to the
remapfunction which simply disables using transformed values from the cache. Then in theresearchfunction caching is turned off.The existing
remapbehavior is maintained as caching by default is turned on.fixes: #393