Accept MAST URIs as input to get_cloud_uris()#3193
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3193 +/- ##
==========================================
+ Coverage 68.34% 68.69% +0.34%
==========================================
Files 231 231
Lines 19190 19214 +24
==========================================
+ Hits 13116 13199 +83
+ Misses 6074 6015 -59 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Tagging @bmorris3 and @dr-rodriguez for visibility |
| extension='png') | ||
|
|
||
| def test_get_cloud_uris_query(self): | ||
| def test_observations_get_cloud_uris_list_input(self): |
There was a problem hiding this comment.
This test triggers a datetime deprecationwarning, could you investigate which upstream package it comes from?
There was a problem hiding this comment.
I wasn't able to reproduce a warning, but I did notice a bug in that test when running it alone that may have been causing some issues? I pushed a fix for it, and hopefully that should work.
There was a problem hiding this comment.
I still see this:
_________________________________ TestMast.test_observations_get_cloud_uris_list_input _________________________________
self = <astroquery.mast.tests.test_mast_remote.TestMast object at 0x12c775850>
def test_observations_get_cloud_uris_list_input(self):
pytest.importorskip("boto3")
uri_list = ['mast:HST/product/u24r0102t_c1f.fits',
'mast:PS1/product/rings.v3.skycell.1334.061.stk.r.unconv.exp.fits']
expected = ['s3://stpubdata/hst/public/u24r/u24r0102t/u24r0102t_c1f.fits',
's3://stpubdata/panstarrs/ps1/public/rings.v3.skycell/1334/061/rings.v3.skycell.1334.'
'061.stk.r.unconv.exp.fits']
# enable access to public AWS S3 bucket
Observations.enable_cloud_dataset()
# list of URI strings as input
uris = Observations.get_cloud_uris(uri_list)
assert len(uris) > 0, f'Products for URI list {uri_list} were not found in the cloud.'
assert uris == expected
# check for warning if filters are provided with list input
> with pytest.warns(InputWarning, match='Filtering is not supported'):
E DeprecationWarning: datetime.datetime.utcnow() is deprecated and scheduled for removal in a future version. Use timezone-aware objects to represent datetimes in UTC: datetime.datetime.now(datetime.UTC).
../../.tox/test-online/lib/python3.12/site-packages/astroquery/mast/tests/test_mast_remote.py:824: DeprecationWarning
There was a problem hiding this comment.
Do you mind sending the package versions you're using? I can't reproduce, and I figure I might be running the test with a newer version of some dependency.
There was a problem hiding this comment.
This is with a fresh tox run, so I'm surprised you don't hit the same problem.
Anyway, it's not in our own codebase, so can just put a blanket ignore in the config file.
There was a problem hiding this comment.
Oh, that might be the issue: I've been running the tests with pytest. I can't quite figure out how to use tox to only run the tests in one file. I've been trying to use tox -e py312-test-alldeps-online -- astroquery/mast/tests/test_mast_remote.py but I keep getting various errors.
146daaf to
5da743b
Compare
5da743b to
60cc19d
Compare
bsipocz
left a comment
There was a problem hiding this comment.
Minor issue with the changelog and the warning filter, the rest looks good to me.
Thanks!
| mast | ||
| ^^^^ | ||
|
|
||
| - Handle coordinates that are not in the ICRS frame in query functions. [#3164] |
| # Datetime deprecation warnings | ||
| ignore::DeprecationWarning:datetime\.datetime |
There was a problem hiding this comment.
This doesn't work as is, besides it's better to be as specific as possible rather than ignoring anything in datetime.datetime
| # Datetime deprecation warnings | |
| ignore::DeprecationWarning:datetime\.datetime | |
| # Triggered in mast, likely boto related | |
| ignore:datetime.datetime.utcnow\(\) is deprecated:DeprecationWarning |
|
Sorry for the delay on this! I just made those changes. |
|
Thanks @snbianco! |
| mast | ||
| ^^^^ | ||
|
|
||
| - Handle a MAST URI string as input for ``Observations.get_cloud_uri`` and a list of MAST URIs as input for | ||
| ``Observations.get_cloud_uris``. [#3193] | ||
|
|
There was a problem hiding this comment.
fyi: I'll move this section down below as we keep this section to highlight potentially breaking API changes rather than for additions/enhacements.
This PR does the following:
Observations.get_cloud_uri.Observations.get_cloud_uris. Logs a warning if the user tries to provide filter criteria as well, since this won't be possible without aTable.MastMissions.