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

Weakmemoize and cleanup #1319

Merged
merged 12 commits into from Nov 27, 2015
Merged

Weakmemoize and cleanup #1319

merged 12 commits into from Nov 27, 2015

Conversation

llllllllll
Copy link
Member

Fixes the leak that we talked about and makes all of the expressions cache their dshape and schema methods.

Also some general cleanup that I saw while making the mostly mechanical changes.

@llllllllll
Copy link
Member Author

tests are failing because of: #1320

@llllllllll llllllllll changed the title Weakmemoize Weakmemoize and cleanup Nov 23, 2015
@cpcloud
Copy link
Member

cpcloud commented Nov 24, 2015

@llllllllll can you restart this build

@llllllllll
Copy link
Member Author

sure, let me rebase away the conflicts too

@cpcloud
Copy link
Member

cpcloud commented Nov 24, 2015

@llllllllll don't rebase yet! i'm reviewing

@llllllllll
Copy link
Member Author

oh, sorry. I will hold off on that.

@cpcloud
Copy link
Member

cpcloud commented Nov 24, 2015

@llllllllll I'm done reviewing if you want to rebase

@cpcloud
Copy link
Member

cpcloud commented Nov 24, 2015

you need to either rebase on top of master or merge it in and resolve the conflicts as well as get the change that fixes the failing test

@llllllllll
Copy link
Member Author

Added a section in the docs on adding new expressions where I cover the _dshape method.

Memory addresses will always be different.
cpcloud added a commit that referenced this pull request Nov 27, 2015
@cpcloud cpcloud merged commit dde5cdb into blaze:master Nov 27, 2015
@cpcloud
Copy link
Member

cpcloud commented Nov 27, 2015

@llllllllll thanks!

@llllllllll
Copy link
Member Author

no, thank you

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

2 participants