Allow documentation tests in FITS verification page

[cmarmo]

Description

This pull request removes the .. doctest-skip directive from the FITS verification page. I could not find a better way to manage warnings than inserting a doctest.IGNORE_EXCEPTION_DETAIL.

Fixes #6686.

[saimn]

Thanks for your work on this @cmarmo, very useful since obviously some examples were out of date. I'm not sure how to best handle the warnings/exception issue (some ideas in the comments). It would be better if the users can see the warnings as they would get them by default.

[pllim]

Re: Warnings -- If you want them to show but don't want doctest to fail, use https://github.com/astropy/pytest-doctestplus#ignoring-warnings

# doctest: +IGNORE_WARNINGS

p.s. If you do this, don't have to write out the warnings in expected outputs.

[saimn]

@pllim - IGNORE_WARNINGS is to hide the warning isn't it ? I would see to see it, since that's the result of the FITS verification.

[pllim]

Yes, correct that IGNORE_WARNINGS does hide it... 🤔

[cmarmo]

Thanks @saimn and @pllim for your suggestions. I have recorded the warnings and printed them: note that one of the warnings is thrown as an exception by default, I've kept the IGNORE_EXCEPTION_DETAIL directive for that one. I'm not sure this is the most readable solution...

[cmarmo]

I've tried to clean a bit, but now to avoid ResourceWarning: unclosed file _io.FileIO warnings all fits files should be closed.

[saimn]

@cmarmo : I've hijacked a bit your PR, sorry, trying to find a better solution for warnings. To keep the docs clear and focused on the initial topic, it would be better if we could avoid all the warnings related code. So currently I have factored the catching and printing in a showwarnings context manager, which works well. And then I think we could move this to pytest-doctestplus in a new doctest flag.

[cmarmo]

@cmarmo : I've hijacked a bit your PR, sorry, trying to find a better solution for warnings. To keep the docs clear and focused on the initial topic, it would be better if we could avoid all the warnings related code. So currently I have factored the catching and printing in a showwarnings context manager, which works well.

@saimn no problem, this kind of refactoring was above my python skills... always happy to learn... :)

And then I think we could move this to pytest-doctestplus in a new doctest flag.

Do you mind if I add a comment in the testsetup block, to document that the function is accessory to the warning management in the example? and that it could one day be replaced by a SHOW_WARNING options?

[saimn]

Do you mind if I add a comment in the testsetup block, to document that the function is accessory to the warning management in the example? and that it could one day be replaced by a SHOW_WARNING options?

@cmarmo - I managed to make a PR on doctestplus (scientific-python/pytest-doctestplus#136), so if you don't mind waiting a bit we can update the PR once it's merged and released ? (hopefully soon!)

[cmarmo]

update the PR once it's merged and released ?

Arrgggh, I should have waited for the release too... :(

[saimn]

I released the new version of pytest-doctestplus, and closed/reopened the PR to trigger the actions builds.

[saimn]

Seems all good now with the new doctestplus version. Thanks @cmarmo !

[cmarmo]

Thank you @saimn .