Skip to content

TST: Get rid of wcs test warnings - #9010

Merged
bsipocz merged 2 commits into
astropy:masterfrom
pllim:wcs-test-warns
Oct 24, 2019
Merged

bsipocz merged 2 commits into
astropy:masterfrom
pllim:wcs-test-warns

Conversation

@pllim

@pllim pllim commented Jul 17, 2019

Copy link
Copy Markdown
Member

This should get rid of most of the wcs test warnings. This is part of work for #7928 . The warnings were captured by removing the line in setup.cfg to suppress pytest warnings and then add this in the same section:

[pytest]
filterwarnings =
    error

And then run tests with python setup.py -P wcs.

Comment thread astropy/wcs/tests/test_wcs.py Outdated
[202.72053428, 47.37893142]])
footprint = w.calc_footprint(axes=axes)
assert_allclose(footprint, ref)
with pytest.raises(FITSFixedWarning):

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

For some of these tests, I need to indent the whole block or Python complains about undefined variable (that was defined inside the block). 🤷‍♀️

from astropy.tests.helper import raises, catch_warnings
from astropy import wcs
from astropy.wcs import _wcs
from astropy.wcs import _wcs # noqa

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

flake8 complains that this import is not used but if I remove it, tests fail. 🤷‍♀️

@pllim
pllim force-pushed the wcs-test-warns branch 3 times, most recently from 2b4132b to 5d09ffd Compare July 17, 2019 20:31
Comment thread astropy/wcs/tests/test_utils.py Outdated
mywcs.wcs.cdelt = cdelt

mywcs.wcs.ctype = ['RA---TAN', 'FREQ']
# Some inputs emit RuntimeWarning

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The warnings here are because the parameters are set incorrectly.
A WCS cannot have both CD and PC defined. A WCS cannot have CD and CDELT defined.
Since the test is not meant to test the warning messages I think the input data is incorrect.
Either fix the inputs to the test and remove warnings or file an issue and I will fix it in a different PR.
I remember seeing some function code that needs to be fixed in a similar way but I have to find it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@nden , I removed the handling of RuntimeWarning. Can we proceed and merge this soon?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I also opened an issue at #9310 as you requested. Thanks!

@nden

nden commented Aug 23, 2019

Copy link
Copy Markdown
Contributor

@pllim Other than that one comment I think this is good.

@pllim

pllim commented Aug 23, 2019

Copy link
Copy Markdown
Member Author

@nden, thanks for the review! I think I addressed your comment. Feel free to approve and/or merge if you are happy with this.

@pllim

pllim commented Aug 23, 2019

Copy link
Copy Markdown
Member Author

I don't think mpldev failures are related.

@bsipocz

bsipocz commented Oct 24, 2019

Copy link
Copy Markdown
Member

I think that everything got addressed from the review here, so I go ahead and merge.

Thanks @pllim!

@bsipocz
bsipocz merged commit 29d0328 into astropy:master Oct 24, 2019
@pllim
pllim deleted the wcs-test-warns branch October 24, 2019 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants