Opened 8 years ago
Last modified 6 years ago
#15056 needs_work enhancement
Bind the wrapped function to the instance to which a `CachedMethodCaller` is bound
Reported by:  SimonKing  Owned by:  

Priority:  major  Milestone:  sage6.8 
Component:  misc  Keywords:  
Cc:  nthiery  Merged in:  
Authors:  Simon King  Reviewers:  
Report Upstream:  N/A  Work issues:  
Branch:  u/SimonKing/bind_the_wrapped_function_to_the_instance_to_which_a__cachedmethodcaller__is_bound (Commits, GitHub, GitLab)  Commit:  016518b2398334101f82937e818f4e276501d205 
Dependencies:  #12601  Stopgaps: 
Description
Currently, a cached method will always be a wrapper of a function. It will bind a CachedMethodCaller
to an instance. However, this CachedMethodCaller
will not bind the wrapped function to the instance. But in some cases, we might want to make the function depend on the instance, see #12978, for example.
Therefore, I suggest to store the bound version of the wrapped function when binding a CachedMethodCaller
to an instance. There might be a slight benefit timewise, too:
sage: def f(self, x): pass sage: m = f.__get__(ZZ, type(ZZ)) sage: %timeit f(ZZ, 3) 1000000 loops, best of 3: 333 ns per loop sage: %timeit m(3) 1000000 loops, best of 3: 320 ns per loop
I make #12601 a dependency, since it touches the same file.
Attachments (1)
Change History (14)
comment:1 Changed 8 years ago by
comment:2 Changed 8 years ago by
 Status changed from new to needs_review
This is now ready for review (and is a prerequisite for #12978).
The underlying idea: We wrap a function f
, resulting in a cached method CMf
. When binding CMf
to an instance inst
(which can be None), then a CachedMethodCaller
or CachedMethodCallerNoArgs
CMCf
is returned, according to the number of arguments. Now, the old behaviour was that CMCf
was holding a copy of f
, and was calling it passing inst
as the first argument.
Now, in #12978, I envision to wrap arbitrary objects f
, in particular objects with a __get__
method, so that f
can be bound to inst
and obviously it is not f
but f bound to inst
what we want to call! As a side effect, we can distinguish cached bound methods of an instance, and cached unbound methods.
Outside of sage/misc/cachefunc.pyx, only little had to change. In fact, there have just been a few places in which the string representation of a cached method appears in a testand the string representation has changed, because it is now Cached version of <bound/unbound method...>
instead of Cached version of <function ...>
.
comment:3 Changed 8 years ago by
Argh. It doesn't work yet. Something in CachedMethodCaller._instance_call
...
comment:4 Changed 8 years ago by
 Status changed from needs_review to needs_work
Changed 8 years ago by
comment:5 Changed 8 years ago by
 Status changed from needs_work to needs_review
The problem was in the __call__
method of CachedMethodCaller
in the case of a CachedInParentMethod
: If we have a CachedInParentMethod
then we need to tell the instance which the cached function needs to be called with.
Anyway, needs review again.
comment:6 Changed 7 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:7 Changed 7 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:8 Changed 7 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:10 Changed 6 years ago by
I try to turn it into a git branch. Isn't git apply
supposed to apply a patch? It somehow didn't work, even though it didn't give me an error message (at least when saying git apply ...
in the folder sage/src
).
comment:11 Changed 6 years ago by
 Branch set to u/SimonKing/bind_the_wrapped_function_to_the_instance_to_which_a__cachedmethodcaller__is_bound
comment:12 Changed 6 years ago by
 Commit set to 016518b2398334101f82937e818f4e276501d205
 Status changed from needs_work to needs_review
I created a git branch manually. Needs review!
New commits:
016518b  Bound and unbound cached methods

comment:13 Changed 6 years ago by
 Milestone changed from sage6.4 to sage6.8
 Status changed from needs_review to needs_work
does not apply, needs rebase
Another nice feature would be: Have cached unbound methods, that will then be called with an additional instance. What I mean is this:
I am only afraid that in the case of
CachedMethodCallerNoArgs
, implementing this feature might imply an inacceptable slowdown. But first experiments show that in the case of several arguments (CachedMethodCaller
) things are fine.