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

FIX isfileobj accepts write-mode files under PY3 #4828

Merged
merged 2 commits into from
Jun 30, 2014

Conversation

ogrisel
Copy link
Contributor

@ogrisel ogrisel commented Jun 26, 2014

Under Python 3, files open in write mode were not recognized as file objects, triggering large memory copies with np.save.

def tempdir():
tmpdir = mkdtemp(prefix="numpy_test_compat")
yield tmpdir
shutil.rmtree(tmpdir)
Copy link
Contributor

Choose a reason for hiding this comment

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

we have another of these in test_io.py, it should probably be moved to testing.utils

@ogrisel
Copy link
Contributor Author

ogrisel commented Jun 30, 2014

@juliantaylor I think I addressed your comments. I rebased as 2 separate commits.

@ogrisel
Copy link
Contributor Author

ogrisel commented Jun 30, 2014

Note: we could have used tempfile.TemporaryDirectory but it's not available in Python 2.6.

@juliantaylor
Copy link
Contributor

unfortunately its also not available in 2.7, so like all python3 stuff maybe we can use it in 2020 (copying it into numpy is also no good idea, ipython did that and it broke when with py3.4)

thanks for the fix, note the commit prefix for bugfixes should be BUG:, the other commit MAINT:

juliantaylor added a commit that referenced this pull request Jun 30, 2014
FIX isfileobj accepts write-mode files under PY3
@juliantaylor juliantaylor merged commit 4e3a24b into numpy:master Jun 30, 2014
@ogrisel
Copy link
Contributor Author

ogrisel commented Jun 30, 2014

Thanks, I'll try to follow the commit conventions next time.

@ogrisel ogrisel deleted the fix-isfileobj-py3 branch June 30, 2014 19:56
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.

None yet

2 participants