Skip to content

Fix warnings in io.fits#8749

Merged
pllim merged 8 commits intoastropy:masterfrom
saimn:fits-warnings
May 24, 2019
Merged

Fix warnings in io.fits#8749
pllim merged 8 commits intoastropy:masterfrom
saimn:fits-warnings

Conversation

@saimn
Copy link
Contributor

@saimn saimn commented May 23, 2019

This removes almost all warnings in io.fits, mostly ResourceWarning, but also a leaking file handle in fitsheader. Ref #7928

The remaining ones are:

====================================== warnings summary =======================================
astropy/io/fits/tests/test_connect.py::test_fits_mixins_qtable_to_table
astropy/io/fits/tests/test_connect.py::test_fits_mixins_as_one[Table]
astropy/io/fits/tests/test_connect.py::test_fits_mixins_as_one[QTable]
astropy/io/fits/tests/test_connect.py::test_fits_mixins_per_column[Table-name_col4]
astropy/io/fits/tests/test_connect.py::test_fits_mixins_per_column[QTable-name_col4]
  /home/simon/dev/astropy/astropy/units/quantity.py:844: DeprecationWarning: elementwise comparison failed; this will raise an error in the future.
    return super().__eq__(other)

astropy/io/fits/tests/test_nonstandard.py::TestNonstandardHdus::test_create_fitshdu_from_filename
  /home/simon/dev/astropy/astropy/io/fits/hdu/nonstandard.py:55: ResourceWarning: unclosed file <_io.FileIO name='/tmp/simon/fits-test-hig3koxp/test.fits' mode='rb' closefd=True>
    return cls.fromhdulist(HDUList.fromfile(filename), compress=compress)

-- Docs: https://docs.pytest.org/en/latest/warnings.html
===================== 723 passed, 9 skipped, 6 warnings in 28.45 seconds ======================

@saimn saimn added this to the v3.2.1 milestone May 23, 2019
@codecov
Copy link

codecov bot commented May 23, 2019 •
edited
Loading

Codecov Report

Merging #8749 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8749      +/-   ##
==========================================
+ Coverage   86.97%   86.97%   +<.01%     
==========================================
  Files         399      399              
  Lines       59373    59384      +11     
  Branches     1100     1100              
==========================================
+ Hits        51639    51651      +12     
+ Misses       7093     7092       -1     
  Partials      641      641
Impacted Files Coverage Δ
astropy/io/fits/scripts/fitsheader.py 91.66% <100%> (+0.54%) ⬆️
astropy/samp/hub.py 77.52% <0%> (+0.13%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2d99bed...50d5bad. Read the comment docs.

pllim
pllim approved these changes May 24, 2019
View reviewed changes
Copy link
Member

@pllim pllim left a comment

Choose a reason for hiding this comment

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

This is much cleaner. Thank you!

p.s. In some of the tests, the close() calls could be done in such a way that they get called even if tests have failed farther up in the function, but that requires turning the function into a test class with teardown methods, which is a battle for another day. It was a close call (pun!).

@pllim pllim modified the milestones: v3.2.1, v3.2 May 24, 2019
@pllim pllim merged commit 33962dd into astropy:master May 24, 2019
@saimn saimn deleted the fits-warnings branch May 24, 2019 12:20
@saimn
Copy link
Contributor Author

saimn commented May 24, 2019

@pllim - Yeah, but if some test fail it means that there is a more important issue ;)

@pllim pllim mentioned this pull request May 24, 2019
Merged
@bsipocz bsipocz modified the milestones: v3.2.1, v3.2 May 24, 2019
bsipocz pushed a commit that referenced this pull request May 27, 2019
@pllim @bsipocz
Merge pull request #8749 from saimn/fits-warnings
0c0b30a 
Fix warnings in io.fits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pllim pllim pllim approved these changes

Assignees

No one assigned

Labels

io.fits no-changelog-entry-needed testing

Projects

None yet

Milestone

v3.2

Development

Successfully merging this pull request may close these issues.

3 participants

@saimn @pllim @bsipocz