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 cloud support for Pan-STARRS mission #2893

Merged
merged 1 commit into from
Dec 7, 2023

Conversation

astrojimig
Copy link

This PR adds functionality to astroquery.mast to enable cloud data access for the Pan-STARRS mission data, which is now available via the MAST public S3 bucket (https://registry.opendata.aws/collab/stsci/)

This is similar to #2261, which were the similar updates for GALEX mission that Jenny did last year.

Thank you!!

@pep8speaks
Copy link

pep8speaks commented Dec 7, 2023

Hello @astrojimig! 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 2023-12-07 22:22:16 UTC

Copy link

codecov bot commented Dec 7, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (d828534) 66.52% compared to head (f55f47b) 66.52%.
Report is 13 commits behind head on main.

Files Patch % Lines
astroquery/mast/cloud.py 0.00% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2893   +/-   ##
=======================================
  Coverage   66.52%   66.52%           
=======================================
  Files         235      235           
  Lines       18101    18116   +15     
=======================================
+ Hits        12041    12052   +11     
- Misses       6060     6064    +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bsipocz bsipocz added the mast label Dec 7, 2023
@bsipocz
Copy link
Member

bsipocz commented Dec 7, 2023

@astrojimig - for fixing the linting I would suggest running the command locally rather than rely on CI, and then you could avoid the multiple linting fixing commits and iterations (could you squash them once all are fixed?). Besides that the PR looks to be in a really nice state, so this will be a rather quick review cycle.

@bsipocz bsipocz added this to the v0.4.7 milestone Dec 7, 2023
@astrojimig
Copy link
Author

@astrojimig - for fixing the linting I would suggest running the command locally rather than rely on CI, and then you could avoid the multiple linting fixing commits and iterations (could you squash them once all are fixed?). Besides that the PR looks to be in a really nice state, so this will be a rather quick review cycle.

Thanks for the tip! And yes, I'll squash all the commits before I mark this as ready for review.

@astrojimig astrojimig marked this pull request as ready for review December 7, 2023 22:26
@bsipocz
Copy link
Member

bsipocz commented Dec 7, 2023

And yes, I'll squash all the commits before I mark this as ready for review.

Thanks!
In general, no need to squash down to 1, but keep it with logical chunks (so just squash out the incremental iterations on one particular thing).

@bsipocz
Copy link
Member

bsipocz commented Dec 7, 2023

Thanks @astrojimig for your first astroquery PR. Looking forward for many more to come!

@bsipocz bsipocz merged commit 9e1017c into astropy:main Dec 7, 2023
9 of 10 checks passed
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

3 participants