Hello @keflavich! 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 2022-03-18 21:36:23 UTC

Codecov Report

Merging #2263 (438e3af) into main (d17805e) will decrease coverage by 0.04%.
The diff coverage is 22.22%.

@@            Coverage Diff             @@
##             main    #2263      +/-   ##
==========================================
- Coverage   62.98%   62.93%   -0.05%     
==========================================
  Files         131      131              
  Lines       17067    17084      +17     
==========================================
+ Hits        10749    10752       +3     
- Misses       6318     6332      +14     

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@keflavich - this is close to being ready once you add a changelog, docs and preferably a test.

@keflavich - I would include this in the release tomorrow if you could add the missing tests and changelog, otherwise will remilestone.

Last sticking point: I raise this warning:

warnings.warn(f"Found cached file {filename} with size {existing_file_length} > expected "
              f"size {length}.  The download is likely corrupted.")

And in the test, I try to test for it:

    caplog.clear()
    with warnings.catch_warnings(record=True) as ww:
        result = alma.download_files(['https://almascience.nao.ac.jp/dataPortal/member.uid___A001_X1284_X1353.qa2_report.html'], verify_only=True)
        assert result
        length = 66336
        existing_file_length = length + 10
        assert f"Found cached file {local_filepath} with size {existing_file_length} > expected size {length}.  The download is likely corrupted." in str(ww)

but it doesn't work:

FAILED astroquery/alma/tests/test_alma_remote.py::test_verify_html_file - AssertionError: assert 'Found cached file /home/adam/.astropy/cache/astroquery/Alma/member.uid___A001_X1284_X1353.qa2_report.html with size 66346 > expected size 66336.  The download is likely corrupted.'...

Adding a print statement gets me:

>>>print(f"WARNING: {str(ww)}")
WARNING: []

Am I doing the catching wrong?

Testing that the code raises warnings is best done with the pytest.warns() function, see https://docs.pytest.org/en/stable/how-to/capture-warnings.html#warns

Thanks @eerovaher, that got it.

I added a new Warning class, which I think is useful - even though it's only presently used here, it could be used generally.

Thanks.

I've also noticed that a few tests are skipped as they are known to fail. I'll come back to those and check how long they take to run, and if it's not super long, I would switch the skip to xfail following the logic in #2326.