Opened 11 years ago

Closed 7 years ago

#12603 closed defect (fixed)

copying cached_methods does not work properly

Reported by: Julian Rüth Owned by: Jason Grout
Priority: minor Milestone: sage-6.4
Component: misc Keywords: cached_method, cache, days71
Cc: Simon King Merged in:
Authors: Julian Rüth Reviewers: David Roe, Aly Deines
Report Upstream: N/A Work issues:
Branch: 90147ca (Commits, GitHub, GitLab) Commit: 90147cae0db60208896a36e2ac3d87c8c2027c63
Dependencies: Stopgaps:

Status badges

Description

A few funny things can happen when copying cached methods:

sage: class A:
...       @cached_method
...       def f(self):
...           return 1
...
sage: class B:
...       g=A.f
...       def f(self):
...           return 2
...       
...
sage: b=B()
sage: b.f()
2
sage: b.g()
1
sage: b.f()
1

The problem is that a call to b.g leads to a new MethodCaller? in b.f; something similar happens here:

sage: class C:
...       g=A.f
...
sage: c=C()
sage: c.g is c.g
False
sage: c.g is c.f
True

Change History (17)

comment:1 Changed 11 years ago by Simon King

Type: PLEASE CHANGEdefect

Is that a new bug (i.e., introduced by my code from #11115)?

Anyway, it is not clear to me how the problem could be solved. Namely, a cached method knows the name under which it was defined, but if one assigns it to a different name afterwards, I see no way for the cached method to find out that new name.

comment:2 Changed 9 years ago by Jeroen Demeyer

Milestone: sage-5.11sage-5.12

comment:3 Changed 9 years ago by For batch modifications

Milestone: sage-6.1sage-6.2

comment:4 Changed 8 years ago by For batch modifications

Milestone: sage-6.2sage-6.3

comment:5 Changed 8 years ago by For batch modifications

Milestone: sage-6.3sage-6.4

comment:6 Changed 7 years ago by Julian Rüth

Branch: u/saraedum/copying_cached_methods_does_not_work_properly

comment:7 Changed 7 years ago by Julian Rüth

Authors: Julian Rüth
Commit: d85263804f48b54e9869914c9a96f9c42253eb9e
Status: newneeds_review

New commits:

d852638Cached methods can not be copied.

comment:8 Changed 7 years ago by David Roe

Status: needs_reviewneeds_work

Need to use ....: doctest continuations rather than ...

comment:9 Changed 7 years ago by git

Commit: d85263804f48b54e9869914c9a96f9c42253eb9e90147cae0db60208896a36e2ac3d87c8c2027c63

Branch pushed to git repo; I updated commit sha1. New commits:

88ca6e0Merge branch 'develop' into t/12603/copying_cached_methods_does_not_work_properly
90147cause correct doctest continuations

comment:10 Changed 7 years ago by Julian Rüth

Status: needs_workneeds_review

comment:11 Changed 7 years ago by David Roe

Looks good. Positive review once the tests all pass.

comment:12 Changed 7 years ago by Alyson Deines

Keywords: days71 added
Status: needs_reviewpositive_review

All tests pass. :)

comment:13 Changed 7 years ago by Volker Braun

Status: positive_reviewneeds_work

Reviewer name

comment:14 Changed 7 years ago by David Roe

Reviewers: David Roe
Status: needs_workpositive_review

Aly, you can add yourself if you like. ;-)

comment:15 Changed 7 years ago by Alyson Deines

Reviewers: David RoeDavid Roe, Aly Deines

comment:16 Changed 7 years ago by Volker Braun

Merge conflict, possibly with #15692

comment:17 Changed 7 years ago by Volker Braun

Branch: u/saraedum/copying_cached_methods_does_not_work_properly90147cae0db60208896a36e2ac3d87c8c2027c63
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.