Skip to content
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

Fix in the cadc module in anticipation of coming changes to the servers #2326

Merged
merged 5 commits into from
Mar 19, 2022

Conversation

andamian
Copy link

Also had to comment out a few int tests with unrelated errors that require further investigation.

The patch is simple, has been tested and it's ready to go.

@codecov
Copy link

codecov bot commented Mar 18, 2022

Codecov Report

Merging #2326 (800006d) into main (3342888) will decrease coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2326      +/-   ##
==========================================
- Coverage   62.98%   62.93%   -0.06%     
==========================================
  Files         131      131              
  Lines       17059    17084      +25     
==========================================
+ Hits        10745    10752       +7     
- Misses       6314     6332      +18     
Impacted Files Coverage Δ
astroquery/cadc/core.py 77.38% <100.00%> (ø)
astroquery/mast/missions.py 76.54% <0.00%> (-2.91%) ⬇️
astroquery/alma/core.py 41.16% <0.00%> (-0.60%) ⬇️
astroquery/exceptions.py 0.00% <0.00%> (ø)
astroquery/mast/services.py 79.56% <0.00%> (ø)
astroquery/mast/collections.py 90.25% <0.00%> (ø)
astroquery/mast/observations.py 76.41% <0.00%> (ø)

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

@bsipocz bsipocz added the cadc label Mar 18, 2022
@bsipocz bsipocz added this to the v0.4.6 milestone Mar 18, 2022
@@ -525,8 +525,11 @@ def get_data_urls(self, query_result, include_auxiliaries=False):
urlencode({'ID': pid_sublist,
'REQUEST': 'downloads-only'}, True)))
for service_def in datalink:
if service_def.semantics == 'http://www.openadc.org/caom2#pkg':
# pkg is an alternative for downloading multiple
if service_def.semantics in \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for line break, long lines up to ~110-120 is fine when they provide better readability

@@ -206,6 +206,7 @@ def test_authsession(self):

@pytest.mark.skipif(one_test, reason='One test mode')
@pytest.mark.skipif(not pyvo_OK, reason='not pyvo_OK')
@pytest.mark.skip('https://github.com/astropy/astroquery/issues/2325')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel leaving these as failing tests are better, it ensures that they are not forgotten. Or maybe change them to xfail, so then we would notice easily if something got fixed upstream.

setup.cfg Outdated
@@ -32,7 +32,7 @@ astropy_header = true
text_file_format = rst
xfail_strict = true
remote_data_strict = true
addopts = --doctest-rst
#addopts = --doctest-rst
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this has to stay, we switched on testing the docs :)

Copy link
Member

@bsipocz bsipocz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I'll fix my comment before merging.

@bsipocz
Copy link
Member

bsipocz commented Mar 19, 2022

Locally I run into timeouts (I suspect my IP might got blacklisted), so merging it without ensuring that the remote tests are all OK.

@bsipocz bsipocz merged commit 44fc1a8 into astropy:main Mar 19, 2022
@bsipocz
Copy link
Member

bsipocz commented Mar 19, 2022

Thanks @andamian!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants