Fix do_pickle of cached_in_parent_method

[saraedum]

The do_pickle parameter was not properly passed on in CachedInParentMethod so that parameter actually never worked (the default was used instead which meant no pickling of caches.)
The doctest did not catch it since the parent was not pickled and unpickled because _parent of the class had been assigned, not of the instance.

Component: misc

Keywords: days85

Author: Julian Rüth

Branch/Commit: 7ea68d7

Reviewer: Florent Hivert

Issue created by migration from https://trac.sagemath.org/ticket/22035

[saraedum]

Branch: u/saraedum/introduce__cached_in_argument_method

[saraedum]

Changed branch from u/saraedum/introduce__cached_in_argument_method to none

[saraedum]

Branch: u/saraedum/introduce__cached_in_argument_method

[saraedum]

New commits:

7162b73

Forward do_pickle to CachedFunction's constructor

[saraedum]

Commit: 7162b73

[sagetrac-msaaltink]

comment:4

This seems like an obvious change and looks like the right thing to do.

There is a small anomaly: because the cache is in the parent, and the parent can be shared my many instances, the do_pickle parameter does not always apply as hoped. Admittedly this is in corner cases, like this one

SP_parent = MyParent()

class SP_pickle(object):
     def parent(self): return SP_parent
     @cached_in_parent_method(do_pickle=True)
     def f(self, x):
          return x

class SP_no_pickle(object):
     def parent(self): return SP_parent
     @cached_in_parent_method(do_pickle=False)
     def f(self, x):
          return x

Here, if instances of SP_pickle and of SP_no_pickle are created, whether the cache is pickled or not depends on which instance's f was called first. I'm not quite sure whether this is a problem, and if so, whether it is bad enough to bother fixing.

[hivert]

Reviewer: Florent Hivert,

[hivert]

comment:5

Replying to @sagetrac-msaaltink:

There is a small anomaly: because the cache is in the parent, and the parent can be shared my many instances, the do_pickle parameter does not always apply as hoped. Admittedly this is in corner cases, like this one

Here, if instances of SP_pickle and of SP_no_pickle are created, whether the cache is pickled or not depends on which instance's f was called first. I'm not quite sure whether this is a problem, and if so, whether it is bad enough to bother fixing.

In my opinion, the fact that the cache should be pickled or not is a choice that should be made made by the parent. Due to the way we write the cached_in_parent decorator, it appears syntactically to be a choice of the element, but I think it really belongs to the parent. As a consequence I won't bother taking care of this very corner case. Actually, I would consider it broken.

Anyway, IHMO the winning argument is that, we don't have that much use cases for cached_in_parent (all of them being related to root systems, by the way...) and even less use cases for parent with two different classes of elements.

So I'm giving a positive review after fixing the merge.

[hivert]

Changed branch from u/saraedum/introduce__cached_in_argument_method to u/hivert/introduce__cached_in_argument_method

[hivert]

Changed commit from 7162b73 to 7ea68d7

[hivert]

New commits:

7ea68d7

Merge branch 'develop' into t/22035/introduce__cached_in_argument_method

[tscrim]

Changed keywords from none to days85

[vbraun]

comment:9

Reviewer name ends with a comma, is that intended? ;-)

[tscrim]

Changed reviewer from Florent Hivert, to Florent Hivert

[vbraun]

Changed branch from u/hivert/introduce__cached_in_argument_method to 7ea68d7