-
-
Notifications
You must be signed in to change notification settings - Fork 408
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mast doc cleanup #1975
Mast doc cleanup #1975
Conversation
bc7ba41
to
65b74ab
Compare
Codecov Report
@@ Coverage Diff @@
## main #1975 +/- ##
=======================================
Coverage 63.08% 63.08%
=======================================
Files 131 131
Lines 17005 17005
=======================================
Hits 10728 10728
Misses 6277 6277 Continue to review full report at Codecov.
|
@bsipocz I rebased, and then also went through and fixed the code block indentation issue. Now it's passing all the tests, but there is a weird extra code block bar at the top of each example (https://astroquery--1975.org.readthedocs.build/en/1975/mast/mast.html#module-astroquery.mast) has this come up with any of Tinuade's other PRs like this? I don't know how to fix it because all the syntax looks right. Ideas? |
@ceb8 - yes, you need to remove all the |
@bsipocz Thank you!!!! |
You mean remotes, too? I still see failures with the rst file. |
Working on the remotes right now. Stay tuned. |
@bsipocz Should be good to go now. |
@@ -142,7 +142,7 @@ To list data missions archived by MAST and avaiable through `astroquery.mast`, u | |||
|
|||
>>> from astroquery.mast import Observations | |||
... | |||
>>> print(Observations.list_missions()) | |||
>>> print(Observations.list_missions()) # doctest: +IGNORE_OUTPUT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may be valuable to keep, as it shouldn't really change that often.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I was unsure about ones like this because the point of the function is to decouple the list of missions from the codebase so that when new missions are added the astroquery code doesn't have to be updated every time. But also, you are correct, this doesn't change that often.
@@ -266,33 +266,31 @@ The below example illustrates downloading all product files with the extension " | |||
|
|||
>>> from astroquery.mast import Observations | |||
... | |||
>>> Observations.download_products('2003839997', | |||
... productSubGroupDescription=["RAW", "UNCAL"], | |||
>>> Observations.download_products('25119363', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this expected to change over time? Basically, I would keep everything output tested that is not expected to change as new data is added to the archive from ongoing missions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think it is likely to change, the archive's linking between products is getting more sophisticated, with the result that more products will likely be returned from the query by obsid. Also the obsids are not guaranteed to stay consistent over time (which is maybe an argument for testing the output anyway so we know when to update the example).
@@ -494,11 +492,11 @@ The returned fields vary by catalog, find the field documentation for specific c | |||
Some catalogs have a maximum number of results they will return. | |||
If a query results in this maximum number of results a warning will be displayed to alert the user that they might be getting a subset of the true result set. | |||
|
|||
.. doctest-remote-data:: | |||
.. doctest-skip:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why switch to skip?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because even though the output includes the max results warning the testing infrastructure considers it an unexpected exception. And obviously I don't want to change the example because the point is to show the user the max results warning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pytest-doctestplus
provides a directive for these kinds of situations, see https://github.com/astropy/pytest-doctestplus#showing-warnings
I'm afraid this now has to be rebased as #2095 has been merged. |
82b85d6
to
556a1c2
Compare
I went ahead and fixed the remaining examples, some were the new mission ones that needed the |
Also, I think the mission examples should not be ignored, but we cannot yet test them in the docs until the bug in #2313 is not fixed. |
Thanks @tinumide and @ceb8! |
No description provided.