Skip to content

[MRG+1]FIX serialization bug in the implementation of the pickle protocol #5560

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

Merged
merged 2 commits into from
Oct 23, 2015

Conversation

arjoly
Copy link
Member

@arjoly arjoly commented Oct 23, 2015

Ping @glouppe @amueller @jmschrei @jnothman

@ngoix this should solve your bug in the isolation forest pull request.

@arjoly arjoly force-pushed the serialization-bug-tree branch from 65893be to d53772d Compare October 23, 2015 13:56
@@ -622,6 +624,8 @@ cdef class Tree:

def __setstate__(self, d):
"""Setstate re-implementation, for unpickling."""
self.capacity = d["capacity"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is overridden below (line 648 in this version of the file). Shall we really have it in the state?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say yes, it's documented in the class attribute.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm no let's remove it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's better to infer those.

@glouppe
Copy link
Contributor

glouppe commented Oct 23, 2015

+1 for merge except for my comment, though it does no harm to add capacity.

@arjoly arjoly changed the title FIX serialization bug in the implementation of the pickle protocol [MRG+1]FIX serialization bug in the implementation of the pickle protocol Oct 23, 2015
@ngoix
Copy link
Contributor

ngoix commented Oct 23, 2015

Yes it works, thanks :) ! +1 for merge.

agramfort added a commit that referenced this pull request Oct 23, 2015
[MRG+1]FIX serialization bug in the implementation of the pickle protocol
@agramfort agramfort merged commit efb0179 into scikit-learn:master Oct 23, 2015
@agramfort
Copy link
Member

great !

@jmschrei
Copy link
Member

Seems I arrived too late to the party on this one.

@arjoly arjoly deleted the serialization-bug-tree branch October 23, 2015 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants