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: fix the weakkeydict key errors #1399

Merged
merged 2 commits into from Jan 26, 2016

Conversation

Projects
None yet
2 participants
@llllllllll
Member

llllllllll commented Jan 26, 2016

I have gotten a bunch of magical mysterious weakkeydict errors. This just uses a non data descriptor instead and replaces the dshape attribute on instance to get the same caching. This removes any strange cache issues and should be faster.

This change is taken from: #1348 but this particular bug is causing me a lot of problems so I am submitting it as its own change.

@llllllllll llllllllll added this to the 0.9.1 milestone Jan 26, 2016

@kwmsmith

This comment has been minimized.

Member

kwmsmith commented Jan 26, 2016

+1: besides fixing the issue, it simplifies the code -- I'm always rusty on weak refs, and when combining weakkeydicts with caching / memoization, the complexity was making other bugfixes difficult.

LGTM.

kwmsmith added a commit that referenced this pull request Jan 26, 2016

Merge pull request #1399 from quantopian/weak-dict-is-weak
BUG: fix the weakkeydict key errors

@kwmsmith kwmsmith merged commit 5568455 into blaze:master Jan 26, 2016

1 check passed

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

@llllllllll llllllllll deleted the quantopian:weak-dict-is-weak branch Jan 26, 2016

@llllllllll

This comment has been minimized.

Member

llllllllll commented Jan 26, 2016

They, the only reason I normally prefer the weakkeydict approach is that the descriptor can be general because you don't need to know the name of the attribute.

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