Repository navigation
TST: Partially fix io.fits test warnings - #7998
Conversation
|
Hi there @pllim 👋 - thanks for the pull request! I'm just a friendly 🤖 that checks for issues related to the changelog and making sure that this pull request is milestoned and labeled correctly. This is mainly intended for the maintainers, so if you are not a maintainer you can ignore this, and a maintainer will let you know if any action is required on your part 😃. Everything looks good from my point of view! 👍 If there are any issues with this message, please report them here. |
4b5982a to
f472687
Compare
saimn
left a comment
saimn
left a comment
There was a problem hiding this comment.
Thanks for the PR @pllim , I have a few hours to kill at the airport so it was perfect to keep me busy for some time ;).
It looks good, I just wonder if we should do this for doctest. These warnings are useful for the users, to explain what happens, so hiding them in the docs is like giving a wrong message. I don't have a better alternative for reducing the number of warnings when running tests though, so not sure which is best.
| tbhdu.dump(datafile, cdfile, hfile) | ||
| tbhdu.dump(datafile, cdfile, hfile, overwrite=True) | ||
| with pytest.warns(AstropyUserWarning, | ||
| match='Overwriting existing file'): |
There was a problem hiding this comment.
This warning could be removed imo, there is no reason to raise a warning if overwrite is True. (I can address this later)
I agree that we shouldn't but I am also not sure how to ask |
well, but it should in general, it should just figure out which ones are expected ;) |
f472687 to
f07660e
Compare
pllim
changed the title
|
Okay, I applied @saimn 's review (thanks, Simon!). Maybe we should merge this one for now and address the rest later, before conflicts start happening? (Assuming tests pass.) |
|
Yes, good idea @pllim, and thanks for addressing this! Hopefully we can find another solution for doctests, maybe with a way to capture warnings with doctest-plus ? |
TST: Partially fix io.fits test warnings
TST: Partially fix io.fits test warnings
TST: Partially fix io.fits test warnings
3 participants
This PR gets rid of some of the warnings seen when removing
addopts = -p no:warningsinsetup.cfgand then runningpython setup.py test -P io.fits --remote-data.There are still a plethora of
ResourceWarningbecause I am not sure how to handle them and I also tried but ran out of energy:zlog.txt
Maybe we can just put this in
setup.cfgand be done with those peskyResourceWarning😱filterwarnings = ignore::ResourceWarningAlso see #7928