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

Add optional 'encoding' argument to import_pickle #723

Merged
merged 2 commits into from Aug 14, 2016

Conversation

Projects
None yet
2 participants
@patricksnape
Contributor

patricksnape commented Aug 9, 2016

This will be ignored on Python 2.x.
Attempts to help solve #722.

Add optional 'encoding' argument to import_pickle
This will be ignored on Python 2.x
@jabooth

This comment has been minimized.

Member

jabooth commented Aug 9, 2016

Yup, this isn't a bad shout.

I basically view this as adding an advanced 'escape hatch' for knowledgable users who know they may cut themselves but at least we provide the option.

If there is even a chance that adding the latin encoding could compromise data, this is about the best balance we can strike (and it certainly reads that way from what we've seen).

If we go down this route, would be good to add to import_pickles() too.

@patricksnape

This comment has been minimized.

Contributor

patricksnape commented Aug 11, 2016

@jabooth Added import_pickles - good to go?

@patricksnape

This comment has been minimized.

Contributor

patricksnape commented Aug 12, 2016

Menpobot is lying - it actually passed.

@patricksnape patricksnape merged commit 2efd451 into menpo:master Aug 14, 2016

1 of 2 checks passed

macOS MenpoBot Jenkins build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@patricksnape patricksnape deleted the patricksnape:optional_pickle_encoding branch Aug 14, 2016

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