Repository navigation
Show warnings in pytest and turn them into errors - #7928
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
well, there was good reasons for adding the ignorance to the config 😅 |
|
Wow. Many seem to be of the type where a test should either be properly written so that it doesn't trigger it, or explicitly look for the warning. Fortunately, there are a lot of duplications (e.g., VOunit test giving warning about a unit being deprecated; ERFA tests warning about bad years, etc.) It does look like we better tackle this module by module... |
|
Adding a "sprint" tag in case people feel like sprinting for this one, one subpackage at a time. |
|
The warnings about figures not being closed by Matplotlib can be fixed by making sure that pytest-mpl is installed (even if it's not actually enabled with |
|
@pllim - can you install pytest-mpl for the job where you are checking the warnings? I think we might want to just add it as one of the packages that gets installed with pytest-astropy so it's always there by default. |
|
re: pytest-mpl -- Didn't it used to be in |
|
@astrofrog -- The build with |
|
We are now at 2148 warnings. Huh. 😕 |
|
hmm, it's worse than when we've started? |
|
😱 I hope I didn't accidentally make things worse in my attempts to fix them. 😱 |
|
Ah, but before we go into full panic. The earlier number was from Python 3.5 run, while the latter one from Python 3.7. If I go back to that earlier log and pick the Python 3.7 build (https://travis-ci.org/astropy/astropy/jobs/443714273), there were actually 2689 warnings. So now we have 541 less. 😬 |
|
Ohh, that's a good point about the python version. I assumed we have more warnings due to the new PRs merged in the past few days not because of the cleanup efforts :) |
|
I need to refresh myself on exactly how this works, but I wonder how many of these warnings come from outside astropy? If you enable warnings using the Python interpreter outside of pytest, you end up getting warnings from all kinds of libraries that we don't really care about. |
…INGS whenever possible
Remove unused code as follow up of astropy#9582 Handle failures cased by numpy/numpy#8945
32dbba3 to
d1cdd1a
Compare
| assert type(t) is iers.IERS_B | ||
|
|
||
| @pytest.mark.remote_data | ||
| @pytest.mark.xfail(reason='See issue 9600') |
There was a problem hiding this comment.
I marked this whole test as xfail until #9600 is fixed. Also reverted my other test modifications to this file, @astrofrog .
|
Thanks for the review everyone! I rebased and addressed the latest round of comments. Unless new warnings were introduced in the commits over the weekend, hopefully we're good to go now. |
|
Thanks @pllim! |
|
Whew. That only took 13 months to clear over 2000 warnings on and off! 😉 It was very much a group effort. Thank you, everyone! |
|
I'm sure at the end it was more than 2000. Thanks for your persistence, it wouldn't have happen without you. |
Show warnings in pytest and turn them into errors
|
Thank you @pllim for leading the charge!! 👏 👍 |
|
Thanks indeed, fantastic!! |
The only thing the file `astropy/coordinates/tests/conftest.py` did was it defined a pytest fixture for converting a deprecation warning into an error, but since astropy#7928 such a fixture is no longer needed.
7 participants
We are ignoring warnings right now but as @bsipocz stated, they should be cleaned up at some point. This PR show them for the purpose of clean-up campaign.
convolution-- TST: Ignore convolution test warnings #7983coordinates-- TST: Ignore coordinates test warnings #7989 and TST: Mark coordinates tests as requiring remote data #8033cosmology-- TST: Fix cosmology test warnings #7992io.ascii-- TST: Fix io.ascii test warnings #7997io.fits-- TST: Partially fix io.fits test warnings #7998 (partial only), Remove warning when overwriting files with BinTableHDU.dump #8045 and Fix warnings in io.fits #8749 by @saimnio.votable-- Implement ability to hide warnings in VO table parsing #8715 by @astrofrog and TST: Get rid of io.votable test warnings #8756modeling-- TST: Ignore test warnings in modeling #9538time-- TST: Get rid of time test warnings #8754timeseries.periodograms-- Fix warnings in timeseries.periodograms #8964units-- Try to ensure we get fewer deprecationwarnings from comparisons. #8755 by @mhvkutils-- TST: Fix warnings in utils tests #8034wcs-- TST: Get rid of wcs test warnings #9010visualization-- TST: Catch warnings in WCSAxes tests #8001 and TST: Fix warnings in visualization #8007 by @astrofrogdoctestplus-- How to specify text_file_format in config? scientific-python/pytest-doctestplus#59Update: Over the months, I might have lost track of some PRs that got merge that got rid of the warnings. I apologize if I missed your PR(s). Please update the list above accordingly if you so desired. See the xref PRs in the comments thread below.
Fun fact: #6163