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.

Sign up for GitHub

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

Jump to bottom

get_obs_products method supports product_type parameter as string or list #2995

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

get_obs_products method supports product_type parameter as string or list #2995

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2d9d5d1
get_obs_products method supports product_type parameter as string or …
Apr 23, 2024
0d8d1fb
align to PEP8
Apr 23, 2024
39a94cf
Modify CHANGES.rst file with get_obs_products method changes
Apr 26, 2024
a041b27
Delete check_list_strings method
davidglt May 6, 2024
e190e95
cleaner get_obs_products method
davidglt May 13, 2024
0b22266
clean whitespaces
davidglt May 13, 2024
04b61de
remote test for product_type parameter as list
davidglt May 20, 2024
d7bcbbc
delete "# doctest: +SKIP" from get_obs_products tests when they use t…
davidglt May 21, 2024
fabe812
fix esa.jwst subsubsection underlining
davidglt May 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGES.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ New Tools and Services
Service fixes and enhancements
------------------------------

esa.jwst
^^^^^^^^

- get_obs_products method supports product_type parameter as string or list [#2995]

mpc
^^^

Expand Down
19 changes: 14 additions & 5 deletions astroquery/esa/jwst/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -965,10 +965,11 @@ def get_obs_products(self, *, observation_id=None, cal_level="ALL",
composite products based on level 2 products). To request upper
levels, please use get_related_observations functions first.
Possible values: 'ALL', 3, 2, 1, -1
product_type : str, optional, default None
List only products of the given type. If None, all products are \
listed. Possible values: 'thumbnail', 'preview', 'auxiliary', \
'science'.
product_type : str or list, optional, default None
List only products of the given type. If None, empty or an element
of the list is empty, all products are listed.
Possible values: 'thumbnail', 'preview', 'auxiliary',
'science', 'info', ['science', 'preview']...
Copy link
Member

Choose a reason for hiding this comment

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

are there any more options? If yes, and we know them, then please list, otherwise drop the .... No need to allow an empty list, the default None should cover that use case.

Copy link
Author

Choose a reason for hiding this comment

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

I used ... because in the list you can provide any combination of 1, 2, 3, 4 or 5 elements, ['science', 'preview'] is just one example of all of them. If you want we can better express this idea.

output_file : str, optional
Output file. If no value is provided, a temporary one is created.

Expand All @@ -978,6 +979,8 @@ def get_obs_products(self, *, observation_id=None, cal_level="ALL",
Returns the local path where the product(s) are saved.
"""

if (type(product_type) is list and '' in product_type) or not product_type:
Copy link
Member

Choose a reason for hiding this comment

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

isinstance(product_type, list) is usually cleaner than what is there, but this is nitpicking. The bigger issue is that this will be True for a list that looks like: ['', 'science', 'info'] which I suppose is not the intended behaviour.

Howeverk duck-typing is real, and the documentation above is clear, so dropping these two lines should be enough. If the user provides a string, or a list that only contains '', it's on to them, the docs lists all the possible values.

Copy link
Author

Choose a reason for hiding this comment

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

With empty element, in the list or in the string, the code will download all products (product_type = None). In your example: ['', 'science', 'info'] is a malformed list so, all products will be downloaded.
We can change "type + is" check to isinstance, no problem

product_type = None
if observation_id is None:
raise ValueError(self.REQUESTED_OBSERVATION_ID)
plane_ids, max_cal_level = self._get_plane_id(observation_id=observation_id)
Expand All @@ -994,10 +997,16 @@ def get_obs_products(self, *, observation_id=None, cal_level="ALL",
max_cal_level=max_cal_level,
is_url=True)
params_dict['planeid'] = plane_ids

if type(product_type) is list:
tap_product_type = ",".join(str(elem) for elem in product_type)
else:
tap_product_type = product_type

self.__set_additional_parameters(param_dict=params_dict,
cal_level=cal_level,
max_cal_level=max_cal_level,
product_type=product_type)
product_type=tap_product_type)
output_file_full_path, output_dir = self.__set_dirs(output_file=output_file,
observation_id=observation_id)
# Get file name only
Expand Down
14 changes: 12 additions & 2 deletions docs/esa/jwst/jwst.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,14 +275,24 @@ To download a data product
>>> output_file = Jwst.get_product(artifact_id='6ab73824-6587-4bca-84a8-eb48ac7251be') # doctest: +SKIP
>>> output_file = Jwst.get_product(file_name='jw01166091001_02102_00002_nrca3_cal.fits') # doctest: +SKIP


To download products by observation identifier, it is possible to use the get_obs_products function, with the same parameters
than get_product_list.
than get_product_list, it also supports product_type parameter as string or list. product_type as string:

.. doctest-remote-data::

>>> from astroquery.esa.jwst import Jwst
>>> observation_id = 'jw01122001001_0210r_00001_nrs2'
>>> results = Jwst.get_obs_products(observation_id=observation_id, cal_level=2, product_type='science') # doctest: +SKIP
>>> results = Jwst.get_obs_products(observation_id=observation_id, cal_level=2, product_type='science')


Here product_type as list:

.. doctest-remote-data::

>>> from astroquery.esa.jwst import Jwst
>>> observation_id = 'jw01122001001_0210r_00001_nrs2'
>>> results = Jwst.get_obs_products(observation_id=observation_id, cal_level=2, product_type=['science', 'preview'])

A temporary directory is created with the files and a list of the them is provided.

Expand Down