Skip to content

Method to get all products by program id (proposal id)#3073

Merged
bsipocz merged 4 commits into
astropy:mainfrom
esdc-esac-esa-int:ESA_jwst-new_astroquery_method_to_get_all_products_by_program_id
Jul 22, 2024
Merged

Method to get all products by program id (proposal id)#3073
bsipocz merged 4 commits into
astropy:mainfrom
esdc-esac-esa-int:ESA_jwst-new_astroquery_method_to_get_all_products_by_program_id

Conversation

@davidglt

Copy link
Copy Markdown
Contributor

Method to get all products by program id (proposal id)

@pep8speaks

pep8speaks commented Jul 22, 2024

Copy link
Copy Markdown

Hello @davidglt! 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 2024-07-22 19:16:42 UTC

davidglt added 2 commits July 22, 2024 09:30
…align it with hubble

Fix the position of the proposal_id mandatory argument in the method definition
@bsipocz bsipocz added this to the v0.4.8 milestone Jul 22, 2024

@bsipocz bsipocz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, thank you! I have just one minor suggestion for a nicer output for the user docs, but your code was in fact the right one that got returned (but as we ignore that output, I would suggest going with the nicer-looking version.)

Comment thread docs/esa/jwst/jwst.rst Outdated
>>> from astroquery.esa.jwst import Jwst
>>> observation_list = Jwst.download_files_from_program(proposal_id='6651', product_type='preview')
>>> print(observation_list) # doctest: +IGNORE_OUTPUT
[np.str_('jw06651001001_05201_00001_nis'), np.str_('jw06651002001_05201_00001_nis')]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Interesting that the values come up wrapped in a np.str_ as opposed to be just strings, and thus leading to this somewhat ugly print as opposed to (of course looping through the values works nicely, but that would be uglier for the docs)
['jw06651001001_05201_00001_nis', 'jw06651002001_05201_00001_nis']

Maybe this can be have a look into sometimes in the future in JwstClass.get_decoded_string?

And here, given that the output is already been ignored, I would suggest to format it nicely.

Suggested change
[np.str_('jw06651001001_05201_00001_nis'), np.str_('jw06651002001_05201_00001_nis')]
['jw06651001001_05201_00001_nis', 'jw06651002001_05201_00001_nis']


return files

def download_files_from_program(self, proposal_id, *, product_type=None, verbose=False):

@bsipocz bsipocz Jul 22, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ahh, one more nitpick: The same method in esa.hubble has the signature:

    def download_files_from_program(self, program, *, folder=None, calibration_level=None,
                                    data_product_type=None, intent=None,
                                    obs_collection=None, instrument_name=None,
                                    filters=None, only_fits=False):

Would you mind making this consistent by using arg names program instead of proposal_id, and also maybe data_product_type instead of product_type? Though the latter doesn't seem to be exactly the same, consistency may be useful.

I'll also consider this as a follow-up item for a new PR, so will merge this as currently is.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@davidglt - not sure you've seen this as I merged the PR as is, without waiting for an API cleanup discussion.

@bsipocz bsipocz force-pushed the ESA_jwst-new_astroquery_method_to_get_all_products_by_program_id branch from bb4600c to 9c1fdf4 Compare July 22, 2024 19:08
@bsipocz bsipocz force-pushed the ESA_jwst-new_astroquery_method_to_get_all_products_by_program_id branch from 9c1fdf4 to 2019702 Compare July 22, 2024 19:16
@bsipocz bsipocz merged commit 91d160f into astropy:main Jul 22, 2024
@esdc-esac-esa-int esdc-esac-esa-int deleted the ESA_jwst-new_astroquery_method_to_get_all_products_by_program_id branch November 25, 2025 08:26
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.

3 participants