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

Prevent astropy warnings in tests #977

Conversation

eslavich
Copy link
Contributor

@eslavich eslavich commented Apr 27, 2021

astropy 4.2.1 started emitting an additional warning when opening bad files. This PR marks tests that open known bad files to ignore that warning.

Resolves #966

@jdavies-st
Copy link
Contributor

Do we know why these are bad files? Almost every test file that the asdf library has written now gets these warnings. There are dozens in the jwst repo as well. 🤔

@eslavich
Copy link
Contributor Author

Do we know why these are bad files?

In asdf at least they're intentionally bad, for example:

def test_no_asdf_header(tmpdir):
    content = b"What? This ain't no ASDF file"

That content isn't recognized as ASDF, so it gets passed to astropy.io.fits, which emits the warning. Do you have examples in jwst of files that should be valid ASDF or FITS but still result in a warning?

@jdavies-st
Copy link
Contributor

Yeah, a ton of them. Start here and scroll up.

https://github.com/spacetelescope/jwst/runs/2422915561?check_suite_focus=true#step:10:845

That said, you bring up a good point. Some are likely bad files, but I've looked at a few, and they are not even writing out files, which makes me think AsdfInFits is generating bad files? Not sure. Would be good to understand whether it's a bug in astropy.io.fits or a bug in asdf.AsdfInFits instead of sweeping it under the rug.

@eslavich
Copy link
Contributor Author

This commit seems responsible:

astropy/astropy@070cd94#diff-6ebde8ab98c018da8dd092e05ef30d9607dc9f998bfed0df91a6b67c671ca651

The warning is emitted from within that _check_padding function. Previously they were raising "Header missing END card." for invalid files before _check_padding. Now the order is reversed so the warnings are appearing before the exception is raised.

Copy link
Contributor

@jdavies-st jdavies-st left a comment

Choose a reason for hiding this comment

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

This looks good.

@eslavich eslavich merged commit 234b91d into asdf-format:master Apr 28, 2021
@eslavich eslavich deleted the eslavich-resolve-astropy-FITS-header-warning branch April 28, 2021 18:55
@jdavies-st
Copy link
Contributor

And let's discuss any issues with jwst elsewhere, preferably in a jwst issue.

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.

Resolve astropy 4.2.1 FITS header warning
2 participants