Repository navigation
Fix FITS convenience functions to actually close files. - #4786
Conversation
Ref astropy#2824 an older bug report. On Python 3 `ResourceWarning` warnings were raised because the FITS convenience methods (`getdata`, `getheader`, `append`, `update`) were not closing files as they were supposed to.
|
@embray - could you review this if you have time? Thanks! |
|
Thanks. You can add bug fix changelog entries in v1.1.3 and IMO v1.0.10. |
|
Once that's done and this is merged you can also close #2824 I think? |
|
Ok, added the changelog entries. |
|
Up ? |
|
|
||
| - ``astropy.io.fits`` | ||
|
|
||
| - Fix convenience functions (`getdata`, `getheader`, `append`, `update`) to |
There was a problem hiding this comment.
This should also be under "bug fix", not "new features", if needed at all. I am not sure why @embray asked for duplicate entries. Usually, it is just under one section...
There was a problem hiding this comment.
Good catch, thanks ! (and fixed)
Not sure either how it works for bug fixes that affect the 2 previous versions.
There was a problem hiding this comment.
also, please use double backticks in the changelog, otherwise they will at some point turn into sphinx warnings...
f3d3dd5 to
7c274d5
Compare
7c274d5 to
1460986
Compare
|
As for the duplicate entries, we've been a little inconsistent about that. Right now we don't have a good process--for bug fixes in particular--to mark what versions it should be backported to. Currently the next "highest version" bug fix release is v1.1.3. But if we are in still maintenance mode for an LTS (the latest being v1.0) then really there should be two milestones: 1.1.3 and 1.0.10. I'm actually not 100% sure if we'll still do another bug fix release for v1.0 because I forget whether or not the LTS support has expired (though I would say it defacto does not expire until a new LTS release is out). So TL;DR wherever possible bug fixes should be backported to both the "current" bugfix release, and the next LTS bugfix release. We don't have a great process in the ticket system right now to indicate that though (I miss Trac). But there should be two changelog entries so that when v1.0.10 is release it's clear at least in its changelog what bugs were fixed. |
|
Thanks for the clarification, @embray ! Since it is approved, I am merging. |
Fix FITS convenience functions to actually close files.
Fix FITS convenience functions to actually close files.
Fix FITS convenience functions to actually close files.
this allows reversing most of astropy#4786 which introduced a regression on how file objects are handled by convenience functions. This fixes the regression and includes a (partial) regression test. Also fixes up exception handling around convenience functions a bit.
this allows reversing most of astropy#4786 which introduced a regression on how file objects are handled by convenience functions. This fixes the regression and includes a (partial) regression test. Also fixes up exception handling around convenience functions a bit.
this allows reversing most of #4786 which introduced a regression on how file objects are handled by convenience functions. This fixes the regression and includes a (partial) regression test. Also fixes up exception handling around convenience functions a bit.
Ref #2824 for an older bug report. On Python 3
ResourceWarningwarnings were raised because the FITS convenience methods (getdata,getheader,append,update) were not closing files as they were supposed to.According to @embray's comment (#2824 (comment)) this could be due to some changes but I didn't find which one.
This was hidden because
ResourceWarningare filtered by default (https://docs.python.org/3.4/library/warnings.html#default-warning-filters), but can be shown by unittest or in my case by nosetest. It seems that they are also hidden with py.test.