Repository navigation
Cleanup uses of prihdr and header in the FITS docs #6643
Description
@MSeifert04 fixed an instance of variable names changing half way through the io.fits docs in one place - however there are other places where this happens, e.g. in the headers.rst file:
Fundamentally the issue is that we are skipping the doctests in all these files, which is bad! We really should fix the examples so they actually run (and with data we can put on data.astropy.org, no excuse really)
I also think header makes more sense than prihdr as a variable name. It would be great to fix this soon since there have been some complaints/confusion.
Reacted by Michael Seifert and Dan D'Avella
Activity
MSeifert04 commentedon Oct 2, 2017
@astrofrog You're right. I actually opened astropy/pytest-remotedata#5 because that would allow to conditionally run the doctests when remote data is given.
I would actually hate to riddle the code with
doctest: +REMOTE_DATAcomments.bsipocz commentedon Oct 2, 2017
This seems to be the duplicate of #6010
saimn commentedon Oct 2, 2017
Yep (I was looking for the issue number).
astrofrog commentedon Oct 2, 2017
@bsipocz - ah yes, though not quite a duplicate, here I mean specifically we need to improve consistency of the variable names, i.e. we could enable doctests and keep inconsistent variable names between pages. This issue is first and foremost about fixing the current names, which we could do before the pytest plugin has what @MSeifert04 needs.
saimn commentedon Oct 2, 2017
For the specific case addressed by #6628, I'm not sure: sometimes
headeris too generic, difficult to search for in a page.prihdris easier to spot, and there is less chance that it is already defined for another unrelated example. So just to say that usingheadereverywhere is maybe not the solution.astrofrog commentedon Oct 2, 2017
@saimn - right, I see your point. Maybe
hdris a solution too (just because it might not be clear what thepristands for). As long as a beginner can follow the examples easily then I don't mind :)bsipocz commentedon Oct 2, 2017
(also, to be honest I feel that @embray had a valid point, and while stuff is still needed to be cleaned up, should complaints like that always trigger the highest priority?)
pllim commentedon Oct 2, 2017
@drdavella was working on making those FITS tests run.
MSeifert04 commentedon Oct 2, 2017
@pllim Thanks for the info, I was just about to start working on this. 😄
pllim commentedon Oct 2, 2017
Not sure if he's still working on it, given ASDF and
pytestwork. But if he's not, at least grab what he has already done first before duplicating work. FYI.astrofrog commentedon Oct 2, 2017
The priority high label is a bit too much I agree, but to me cleaning up the docs should actually take precedence on internal code cleanup. I just wanted to make sure this didn't get ignored (it worked!).
drdavella commentedon Oct 2, 2017
@pllim, @MSeifert04, unfortunately this is near the bottom of my to-do list at the moment. I pushed some work to https://github.com/drdavella/astropy/tree/enable-fits-doctest. I didn't get too far with it, but maybe it's a good starting point.
MSeifert04 commentedon Oct 2, 2017
thanks for the fast reply @drdavella , I'll have a look at your PR and see how far I can get 😄
saimn commentedon Oct 7, 2017
Closing since #6664 addressed this (using
hdrinstead ofprihdr).