Opened 9 years ago

Closed 5 years ago

#12603 closed defect (fixed)

copying cached_methods does not work properly

Reported by: saraedum Owned by: jason
Priority: minor Milestone: sage-6.4
Component: misc Keywords: cached_method, cache, days71
Cc: SimonKing 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 9 years ago by SimonKing

  • Type changed from PLEASE CHANGE to defect

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 8 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:3 Changed 8 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:4 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:5 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:6 Changed 6 years ago by saraedum

  • Branch set to u/saraedum/copying_cached_methods_does_not_work_properly

comment:7 Changed 6 years ago by saraedum

  • Authors set to Julian Rüth
  • Commit set to d85263804f48b54e9869914c9a96f9c42253eb9e
  • Status changed from new to needs_review

New commits:

d852638Cached methods can not be copied.

comment:8 Changed 6 years ago by roed

  • Status changed from needs_review to needs_work

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

comment:9 Changed 6 years ago by git

  • Commit changed from d85263804f48b54e9869914c9a96f9c42253eb9e to 90147cae0db60208896a36e2ac3d87c8c2027c63

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 6 years ago by saraedum

  • Status changed from needs_work to needs_review

comment:11 Changed 6 years ago by roed

Looks good. Positive review once the tests all pass.

comment:12 Changed 5 years ago by aly.deines

  • Keywords days71 added
  • Status changed from needs_review to positive_review

All tests pass. :)

comment:13 Changed 5 years ago by vbraun

  • Status changed from positive_review to needs_work

Reviewer name

comment:14 Changed 5 years ago by roed

  • Reviewers set to David Roe
  • Status changed from needs_work to positive_review

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

comment:15 Changed 5 years ago by aly.deines

  • Reviewers changed from David Roe to David Roe, Aly Deines

comment:16 Changed 5 years ago by vbraun

Merge conflict, possibly with #15692

comment:17 Changed 5 years ago by vbraun

  • Branch changed from u/saraedum/copying_cached_methods_does_not_work_properly to 90147cae0db60208896a36e2ac3d87c8c2027c63
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.