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

astroquery.mast : adding GALEX mission Cloud support #2261

Merged
merged 4 commits into from
Feb 13, 2022

Conversation

jaymedina
Copy link
Contributor

Adding cloud support for the GALEX mission.

@codecov
Copy link

codecov bot commented Jan 24, 2022

Codecov Report

Merging #2261 (a82fc2f) into main (08e57d7) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

❗ Current head a82fc2f differs from pull request most recent head 8711b84. Consider uploading reports for the commit 8711b84 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2261      +/-   ##
==========================================
- Coverage   62.75%   62.74%   -0.01%     
==========================================
  Files         130      130              
  Lines       16835    16837       +2     
==========================================
  Hits        10564    10564              
- Misses       6271     6273       +2     
Impacted Files Coverage Δ
astroquery/mast/cloud.py 22.07% <0.00%> (-0.59%) ⬇️

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 08e57d7...8711b84. Read the comment docs.

@jaymedina
Copy link
Contributor Author

jaymedina commented Jan 31, 2022

result = Observations.download_file(uri, cloud_only=True)
ERROR: Error pulling from S3 bucket: 'productFilename' [astroquery.mast.observations]
WARNING: Skipping file... [astroquery.mast.observations]

Preventing file skip shows that the issue is in get_cloud_uri:

astroquery/astroquery/mast/cloud.py in get_cloud_uri(self, data_product, include_bucket, full_url)
    130                 raise
    131 
--> 132         warnings.warn("Unable to locate file {}.".format(data_product['productFilename']), NoResultsWarning)
    133         return None
    134 

KeyError: 'productFilename'

And yeah, it just looks like the data isn't there yet:

from astroquery.mast import cloud

ca = cloud.CloudAccess()
ca.get_cloud_uri(galex_products[1])

WARNING: NoResultsWarning: Unable to locate file [Jenny edit: CENSORED FOR SECURITY]. [astroquery.mast.cloud]

The test below shows galex will be supported for the cloud with the changes in this PR, so I think all that's left now is to way for the GALEX data to be uploaded into the cloud and I will check back on this PR to re-run tests and troubleshoot if needed:

ca.is_supported(data_product=galex_products[0])

Out[27]: True

A separate issue is how the data_product for supported missions has been working with get_cloud_uri all this time when get_cloud_uri only takes Astropy tables but data_product has always been a python dict.

data_product is created here, with data_uri being the keyword value:

data_product = {'dataURI': uri}

Then data_product is fed into here:

def download_file(self, data_product, local_path, cache=True):

Which feeds it into get_cloud_uri in the same form it came in (a python dict):

bucket_path = self.get_cloud_uri(data_product, False)

But get_cloud_uri takes only Astropy rows:

data_product : `~astropy.table.Row`

Which is why I'm getting the productFilename key error because it's trying to get a column out of my python dict. Need to backtrack and watch things run with a Tess or hst input and see if I missed a stepped where that conversion happens.

@bsipocz
Copy link
Member

bsipocz commented Feb 10, 2022

It's not much help for you for this PR, but ultimately I think we need to make these download functions a bit more generic/ or rewrite them and put into utils, so eventually the same approach will work for all three archives.

@jaymedina jaymedina force-pushed the add-galex branch 3 times, most recently from 90eade3 to f68d01e Compare February 12, 2022 02:16
@jaymedina jaymedina marked this pull request as ready for review February 12, 2022 02:19
@jaymedina
Copy link
Contributor Author

jaymedina commented Feb 12, 2022

I think this is ready to go. Based on convos with the team, it sounds like this is a high priority one, so should take precedent over #2279 . Thanks!

@jaymedina jaymedina changed the title [WIP] astroquery.mast : adding GALEX mission Cloud support astroquery.mast : adding GALEX mission Cloud support Feb 12, 2022
… download. this will ensure there are GALEX results in the db queries and that cloud functionality is up to date.
… cloud.py and download the proper file. updating style and format and added a changelog entry.
@ceb8 ceb8 merged commit ed4bcbe into astropy:main Feb 13, 2022
@ceb8 ceb8 added this to the v0.4.6 milestone Feb 13, 2022
@ceb8
Copy link
Member

ceb8 commented Feb 13, 2022

Thanks @jaymedina!

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

Successfully merging this pull request may close these issues.

None yet

4 participants