Conversation
kaushikcfd
force-pushed
the
memoized_mappers
branch
from
fe82705 to
367da3d
Compare
kaushikcfd
force-pushed
the
memoized_mappers
branch
7 times, most recently
from
b0db09e to
1a624be
Compare
kaushikcfd
force-pushed
the
memoized_mappers
branch
2 times, most recently
from
26c5f17 to
a697d47
Compare
matthiasdiener
force-pushed
the
memoized_mappers
branch
from
aba027a to
1d0767f
Compare
inducer
left a comment
inducer
left a comment
There was a problem hiding this comment.
Thanks! Sorry this sat so long, it fell through the cracks somehow. A few things below, hopefully straightforward to deal with.
pymbolic/mapper/__init__.py
Outdated
| A cache key implementation assuming *expr* is immutable while elements | ||
| of *args*, *kwargs* aren't. | ||
| """ | ||
| return ((id(expr), expr), args, tuple(sorted(kwargs.items()))) |
There was a problem hiding this comment.
Use immutables for hashing? (but see below)
There was a problem hiding this comment.
Don't think there's any difference: https://gist.github.com/kaushikcfd/3b9656f5ce0f813044966309ea472edc
kaushikcfd
force-pushed
the
memoized_mappers
branch
3 times, most recently
from
f1373c7 to
ad51ea5
Compare
|
LGTM! In it goes. (Some doc tweaks in dc96d2a.) |
| class CachedSubstitutionMapper(CachedIdentityMapper, | ||
| SubstitutionMapper): | ||
| def __init__(self, subst_func): | ||
| CachedIdentityMapper.__init__(self) | ||
| SubstitutionMapper.__init__(self, subst_func) |
There was a problem hiding this comment.
Does this actually return from the cache for map_variable, map_subscript, map_lookup?
There was a problem hiding this comment.
I was just wondering because the SubstitutionMapper from which this is derived directly calls the IdentityMapper for these methods. But if I understood the MRO here correctly, this appears to not be an issue.
There was a problem hiding this comment.
SubstitutionMapper from which this is derived directly calls the IdentityMapper for these methods.
Cache lookups happen in rec, but SubstitutionMapper doesn't override rec. I think this is OK, but maybe I'm missing something.
3 participants
Closes #89.