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

Pass additional kwargs to pickle.load (in __load) #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

stsievert
Copy link
Contributor

No description provided.

@raamana
Copy link
Owner

raamana commented Oct 17, 2017

Thanks Scott for the PR - could you elaborate on the use cases for this? Perhaps give me an example.

@stsievert
Copy link
Contributor Author

Any of the use cases supported by the keyword args for pickle.load.

I seem to remember some headache with saving under Python 2 and loading under Python 3 (or something like that). pickle.load did not have any kwargs under Python 2 (source) but does under Python 3 (source).

@raamana
Copy link
Owner

raamana commented Oct 18, 2017

I see, good idea. MLDataset.load must also go hand in hand with MLDataset.save to be complete and useful!

Would you have time to add that that too and add a few tests to ensure they work well together?

If possible try to make it work python 2.7 as well, but if its not possible, its not critical.

@stsievert
Copy link
Contributor Author

If possible try to make it work python 2.7 as well, but if its not possible, its not critical.

Does this not work under Python 2.7?

Would you have time to add that that too and add a few tests to ensure they work well together?

I've added passing kwargs on in MLDataset.save, but don't have time to write tests. If you're constrained and can't write files in your tests, I'd look into using a StringIO object.

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.

2 participants