Skip to content

Conversation

@raulgarreta
Copy link
Contributor

Fix for issue #1332

Copy link
Member

Choose a reason for hiding this comment

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

...which is more efficient on objects that carry large numpy arrays internally as is often the case for fitted scikit-learn estimators.

@ogrisel
Copy link
Member

ogrisel commented Apr 17, 2014

Apart from my comments, this looks good. +1 for merge once they are addressed.

@raulgarreta
Copy link
Contributor Author

Addressed ogrisel comments in 640bff9

@ogrisel
Copy link
Member

ogrisel commented Apr 17, 2014

Ping @mblondel @amueller @GaelVaroquaux

@ogrisel ogrisel changed the title [MRG] added a new section on model persistence [MRG+1] added a new section on model persistence Apr 17, 2014
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) when pulling 640bff9 on raulgarreta:model_persistence_doc into b70a481 on scikit-learn:master.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure about this positioning. For example, after (or within) model selection and evaluation seems more coherent.

Copy link
Member

Choose a reason for hiding this comment

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

+1

@mblondel
Copy link
Member

For the sake of consistency, I'd rather avoid the use of "you": we don't address the reader directly in the rest of the doc. Besides that +1 for merge once @jnothman 's comments have been taken into account.

@jnothman
Copy link
Member

Fixed by #3317

@jnothman jnothman closed this Jun 27, 2014
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