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

[davidglt]

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

[pep8speaks]

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

[bsipocz]

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.)

[bsipocz]

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']

[bsipocz]

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.

[bsipocz]

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