Ticket #3781 (closed enhancement: fixed)

Opened 5 years ago

Last modified 5 years ago

[with patch, with positive review] add a cached_method decorator

Reported by: mhansen Owned by: cwitty
Priority: minor Milestone: sage-3.1
Component: misc Keywords:
Cc: Work issues:
Report Upstream: Reviewers:
Authors: Merged in:
Dependencies: Stopgaps:

Description (last modified by mabshoff) (diff)

mhansen: Does anyone feel up for reviewing #3781 for me?
[4:21pm] ncalexan: I'll look at it, one moment.  I've wanted 
this for a while.
[4:22pm] mhansen: Awesome.  It doesn't work on C extension 
types though since they don't have a __dict__.  This could 
be done by storing the cache in the decorator object with 
a weakref though.
[4:22pm] ncalexan: The problem is much more complicated 
than this.
[4:23pm] ncalexan: Okay, there are other problems too, 
like un-hashable arguments will break it.
[4:23pm] mhansen: Yep
[4:23pm] ncalexan: And there is no way to clear the cache...
[4:23pm] ncalexan: And the tests don't actually demonstrate 
that the cache is workin.
[4:24pm] ncalexan: (One could touch the cache with an 
incorrect answer, then verify it is "correctly" returning 
that value)
[4:25pm] ncalexan: For what it is, though, it's fine.  It 
will hurt nothing -- shall I review positive?
[4:26pm] mhansen: If you could, that'd be great.  I do 
know it's limitations, but there are some big patches 
going in that depend on it.  I'll make a ticket with 
your comments for improvement.
[4:28pm] ncalexan: How big are the big patches?  In fact, 
I don't care -- this declares the intent nicely and can be 
upgraded independently later.  One moment.

Attachments

trac_3781.patch Download (3.3 KB) - added by mhansen 5 years ago.

Change History

Changed 5 years ago by mhansen

comment:1 Changed 5 years ago by ncalexan

  • Description modified (diff)
  • Summary changed from [with patch, needs review] add a cached_method decorator to [with patch, with positive review] add a cached_method decorator

comment:2 Changed 5 years ago by mabshoff

  • Description modified (diff)

comment:3 Changed 5 years ago by mabshoff

  • Status changed from new to closed
  • Resolution set to fixed

Merged in Sage 3.1.alpha1

Note: See TracTickets for help on using tickets.