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

Set _main_module when extending StockPickler #24

Merged
merged 2 commits into from
Mar 4, 2014

Conversation

numpand
Copy link

@numpand numpand commented Feb 25, 2014

#23

@mmckerns
Copy link
Member

I will accept this. Please make your test 3.x compatible.

try:
    from StringIO import StringIO
except ImportError:
    from io import BytesIO as StringIO

@numpand
Copy link
Author

numpand commented Feb 28, 2014

Just to be clear, do you want me to do another commit with the above change?

@mmckerns
Copy link
Member

yes. you guys had mentioned possibly making a future pull request, so I wanted to also make sure that you were aware that dill is 3.x compatible. I appreciate the pull, and think it'd be cleaner if you made a second commit with the above change. Thanks, and I look forward to seeing what you guys have in mind. (Are you part of PISM like @citibob? That seems like an interesting project -- I have some frameworks that might apply well to PISM, see https://github.com/uqfoundation.)

@numpand
Copy link
Author

numpand commented Mar 3, 2014

I modified the test to be 3.x compatible (I did not test with Python 3.x, but hopefully the change is small enough that it should not be an issue). I am not part of PISM btw.

mmckerns added a commit that referenced this pull request Mar 4, 2014
Set _main_module when extending StockPickler
@mmckerns mmckerns merged commit 6280dd0 into uqfoundation:master Mar 4, 2014
@mmckerns
Copy link
Member

mmckerns commented Mar 4, 2014

@numpand: Thanks, I appreciate the follow-up. I tested it against 3.x and the rest of the test suite, and it passes.

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

Successfully merging this pull request may close these issues.

2 participants