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

BUG: Removed unnecessary storage of args as instance variables in Rbf #3861

Merged
merged 1 commit into from
Aug 17, 2014

Conversation

jetuk
Copy link
Contributor

@jetuk jetuk commented Aug 5, 2014

A small change to Rbf.__call__ which means that the incoming arrays are no longer stored on self. This is an optimization for memory usage so that args in 'xa' are not stored between calls to Rbf.

As far as I can see the usage of self.xa after a call is not documented, and this change passes existing tests. I have no idea how to write a test for this memory consumption.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 8a93102 on jetuk:master into 13a73f4 on scipy:master.

@rgommers rgommers added the PR label Aug 12, 2014
@rgommers
Copy link
Member

@jetuk no test is needed here. If we indeed consider xa to not be a public attribute (and I agree with you that it doesn't look like that), then this PR is fine as is.

@rgommers
Copy link
Member

Any users of Rbf that see any value in this stored xa intermediate result?

@rgommers
Copy link
Member

If no responses within a week or so, I think I'll merge this PR.

@pv pv removed the PR label Aug 13, 2014
@rgommers rgommers added this to the 0.15.0 milestone Aug 17, 2014
rgommers added a commit that referenced this pull request Aug 17, 2014
BUG: Removed unnecessary storage of args as instance variables in Rbf
@rgommers rgommers merged commit 2286b43 into scipy:master Aug 17, 2014
@rgommers
Copy link
Member

Merged, thanks @jetuk.

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

Successfully merging this pull request may close these issues.

None yet

4 participants