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

Conversation

Projects
None yet
2 participants
@llllllllll
Member

llllllllll commented Nov 23, 2015

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 llllllllll force-pushed the quantopian:weakmemoize branch from 61e5e62 to 1b6b4ca Nov 23, 2015

@llllllllll llllllllll added this to the 0.9.0 milestone Nov 23, 2015

@llllllllll llllllllll force-pushed the quantopian:weakmemoize branch from 1f179a8 to d81466c Nov 23, 2015

@llllllllll

This comment has been minimized.

Member

llllllllll commented Nov 23, 2015

tests are failing because of: #1320

@llllllllll llllllllll changed the title from Weakmemoize to Weakmemoize and cleanup Nov 23, 2015

@cpcloud

This comment has been minimized.

Member

cpcloud commented Nov 24, 2015

@llllllllll can you restart this build

@llllllllll

This comment has been minimized.

Member

llllllllll commented Nov 24, 2015

sure, let me rebase away the conflicts too

@cpcloud

This comment has been minimized.

Member

cpcloud commented Nov 24, 2015

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

@llllllllll

This comment has been minimized.

Member

llllllllll commented Nov 24, 2015

oh, sorry. I will hold off on that.

@cpcloud

This comment has been minimized.

Member

cpcloud commented Nov 24, 2015

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

@cpcloud

This comment has been minimized.

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 llllllllll force-pushed the quantopian:weakmemoize branch from d81466c to b718685 Nov 24, 2015

@llllllllll

This comment has been minimized.

Member

llllllllll commented Nov 24, 2015

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

@llllllllll llllllllll force-pushed the quantopian:weakmemoize branch from e6a72cc to c3bf874 Nov 24, 2015

DOC: cut example because it won't be meaningful
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

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@cpcloud

This comment has been minimized.

Member

cpcloud commented Nov 27, 2015

@llllllllll thanks!

@llllllllll

This comment has been minimized.

Member

llllllllll commented Nov 28, 2015

no, thank you

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