astroquery.mast.Observations get_cloud_uri(s) update

[jaymedina]

Updating the get_cloud_uri/s methods to access the cloud anonymously using the inherited enable_cloud_access method from MastQuerywithLogin.

[jaymedina]

I've just joined the astropy project so this should be the last time I need manual approval to run checks.

[codecov]

Codecov Report

Merging #2168 (5b6d5b1) into main (729cf6d) will decrease coverage by 4.62%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #2168      +/-   ##
==========================================
- Coverage   66.43%   61.80%   -4.63%     
==========================================
  Files         418      129     -289     
  Lines       28032    16287   -11745     
==========================================
- Hits        18622    10066    -8556     
+ Misses       9410     6221    -3189     

Impacted Files	Coverage Δ
astroquery/mast/observations.py	75.89% <0.00%> (ø)
astroquery/jplhorizons/core.py	64.49% <0.00%> (-2.18%)	⬇️
astroquery/utils/schema.py	66.42% <0.00%> (-1.46%)	⬇️
astroquery/esasky/core.py	18.28% <0.00%> (-1.43%)	⬇️
astroquery/atomic/core.py	36.88% <0.00%> (-1.02%)	⬇️
astroquery/utils/tap/gui/login.py	23.65% <0.00%> (-0.82%)	⬇️
...roquery/ipac/nexsci/nasa_exoplanet_archive/core.py	68.87% <0.00%> (-0.58%)	⬇️
astroquery/astrometry_net/core.py	50.00% <0.00%> (-0.53%)	⬇️
astroquery/vo_conesearch/core.py	39.75% <0.00%> (-0.49%)	⬇️
astroquery/esa/xmm_newton/core.py	60.46% <0.00%> (-0.41%)	⬇️
... and 307 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 729cf6d...5b6d5b1. Read the comment docs.

[jaymedina]

I see a message from Codecov saying Added line #L756 was not covered by tests , and the same for 758. Is this referring to the test_mast.py script under tests/ ? How do I clear up this warning?

[bsipocz]

@jaymedina - we're not strict on codecov, as in many cases it reports nonsense, we mostly use it to monitor that test coverage is not dropping, in practice maintainers check the diff and those reports and ask for more tests if something is clearly not covered in a PR.
But in this case, those lines are indeed not covered by either mock or remote tests. If you can think of a way to run tests for the cloud parts, without sharing credentials in a public repo, well, that would be a great addition.

(Btw, you can also run the remote tests locally, pytest setup.py -R -P mast python setup.py -R -P mastis the command. There are a few currently failing for mast, totally unrelated to this PR, and as I see most only need an update into their asserts, or maybe rethink a logic of what we do check on. )

[bsipocz]

Ahh, OK, my bad. So, if anonymous access is indeed possible and free, then I would suggest adding a test that just does that. Originally I think it was the case I listed above that we should have handled credentials somehow. The warning would not disappear from the diff page here, but nevertheless, the remote tests would cover the functionality.

[pep8speaks]

Hello @jaymedina! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-11-12 22:26:12 UTC

[jaymedina]

OK thanks, and yes I can write up a test for this. I would put it in the test_mast.py under tests right?

[bsipocz]

I would put it in the test_mast.py under tests right?

no, it should be in astroquery/mast/tests/test_mast_remote.py

[jaymedina]

Sounds good!

[jaymedina]

I'm getting an error when running my unit test on pytest:

ERROR: usage: pytest [options] [file_or_dir] [file_or_dir] [...] pytest: error: unrecognized arguments: --doctest-rst inifile: /mypath/astroquery/setup.cfg rootdir: /mypath//astroquery

Do you recognize this error? @bsipocz

Edit: This is my command
pytest test_mast_remote.py::test_get_cloud_uri

[bsipocz]

Make sure you have all the plugins installed and up-to-date (components of pytest-astropy). Also, note the typo I made in the command above, test via pytest may not work as easily, but do use python setup.py -R -P mast instead.

Also, I would suggest to write a test function with working syntax before committing it, as it will take a lot of iterations and longer if you develop it directly in the codebase. (E.g. you can do it in an ipython session, or something similar).

[jaymedina]

Thanks, I was able to get pytest working and my cloud unit tests ran successfully. A couple things:

1. I was able to get pytest running by installing the plugins as you said, and following the documentation here: https://github.com/astropy/pytest-remotedata
2. I was unable to get tests to run using the python setup.py -R -P mast command, even tho I've downloaded all the plugins by installing this package: https://github.com/astropy/pytest-astropy
3. While my unit tests ran successfully, a couple of other unit tests broke:

================================================================================================================= short test summary info ==================================================================================================================
FAILED test_mast_remote.py::TestMast::test_observations_get_product_list - assert 27 == 30
FAILED test_mast_remote.py::TestMast::test_observations_download_products - astroquery.exceptions.NoResultsWarning: Query returned no results.
FAILED test_mast_remote.py::TestMast::test_observations_download_file - astroquery.exceptions.NoResultsWarning: Query returned no results.
FAILED test_mast_remote.py::TestMast::test_catalogs_query_region - astroquery.exceptions.MaxResultsWarning: Maximum catalog results returned, may not include all sources within radius.
FAILED test_mast_remote.py::TestMast::test_catalogs_query_criteria - assert 316 >= 400
FAILED test_mast_remote.py::TestMast::test_tesscut_get_sectors - astroquery.exceptions.NoResultsWarning: Coordinates are not in any TESS sector.
FAILED test_mast_remote.py::TestMast::test_zcut_get_surveys - astroquery.exceptions.NoResultsWarning: Coordinates are not in an available deep field survey.
FAILED test_mast_remote.py::TestMast::test_zcut_download_cutouts - astroquery.exceptions.NoResultsWarning: No data at cutout size/location. Cutout not performed.
FAILED test_mast_remote.py::TestMast::test_zcut_get_cutouts - astroquery.exceptions.NoResultsWarning: No data at cutout size/location. Cutout not performed.
========================================================================================================= 9 failed, 30 passed in 186.62s (0:03:06) =========================================================================================================

I've made an issue for this (#2172) and can look into fixing these after I wrap up our instance of unit tests.

[jaymedina]

Let me know if there's anything else that needs updating before merge

cc: @tomdonaldson

[bsipocz]

We don't necessarily write assert messages in the tests, but you're right, they likely don't hurt to have :)

[bsipocz]

Thanks @jaymedina!