Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add a cached_method decorator #3781

Closed
mwhansen opened this issue Aug 6, 2008 · 4 comments
Closed

add a cached_method decorator #3781

mwhansen opened this issue Aug 6, 2008 · 4 comments

Comments

@mwhansen
Copy link
Contributor

mwhansen commented Aug 6, 2008

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.

Component: misc

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

@ncalexan
Copy link
Mannequin

ncalexan mannequin commented Aug 6, 2008

comment:1

Attachment: trac_3781.patch.gz

@ncalexan

This comment has been minimized.

@ncalexan ncalexan mannequin added the s: positive review label Aug 6, 2008
@sagetrac-mabshoff

This comment has been minimized.

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Aug 8, 2008

comment:3

Merged in Sage 3.1.alpha1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant