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

Close fd temp file following rec2csv_bad_shape test #971

Merged
merged 1 commit into from Jul 2, 2012

Conversation

jenshnielsen
Copy link
Member

This avoids a warning in python3 about the unclosed file
and potential side effects of the open file.

@WeatherGod
Copy link
Member

Shouldn't it close when the object goes out of scope anyway? I don't see any harm in including this code (the ValueError exception should still bubble up to nose). I just wanted to make sure I understood tempfile correctly.

@jdh2358
Copy link
Collaborator

jdh2358 commented Jun 30, 2012

If this is a fix for python3, shouldn't it go against master rather than 1.1.x, which doesn't support python3?

@jenshnielsen
Copy link
Member Author

Well I can rebase it against master and thats fine with me, but I believe the behavior is the same on the different python versions the only difference is that python 3 gives the not closed warning while python2 does not, thus it seems like a good idea to close the temp file in any case.
I think the tempfile is closed when it goes out of scope but I think the main point is that the file is destroyed when it is closed. http://docs.python.org/library/tempfile.html

@mdboom
Copy link
Member

mdboom commented Jul 1, 2012

+1

@WeatherGod
Copy link
Member

In any case, explicit is better than implicit. I am fine with it being merged in.

mdboom added a commit that referenced this pull request Jul 2, 2012
Close fd temp file following rec2csv_bad_shape test
@mdboom mdboom merged commit 2c85848 into matplotlib:v1.1.x Jul 2, 2012
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

4 participants