-
-
Notifications
You must be signed in to change notification settings - Fork 417
Euclid: new method to retrieve scientific LE3 products #3313
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
base: main
Are you sure you want to change the base?
Euclid: new method to retrieve scientific LE3 products #3313
Conversation
… Pre-recon', into 'SEL Wide'
5ec7e07
to
8c96881
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3313 +/- ##
==========================================
+ Coverage 69.87% 69.97% +0.09%
==========================================
Files 232 232
Lines 19757 19818 +61
==========================================
+ Hits 13806 13867 +61
Misses 5951 5951 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3a2b771
to
f0ee810
Compare
92cdb46
to
ea52f13
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good with some minor, mostly documentation-related comments.
Also, I would think a rebase would be great to remove the merge commit as well as the unrelated changes in e.g. the changelog commit.
astroquery/esa/euclid/core.py
Outdated
product_type : str, optional, default None | ||
Available product types per category and group: | ||
|
||
#. Internal Data Products |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ~200 lines feels to be a lot, maybe we should instead refer to a docs URL in this docstring?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Euclid documentation is pretty complex, so we tried to provide a simple description of the available parameters that could be used. Although this information was extracted from the Euclid documentation, it is not straight forward to extract the category/group/product_type for all the LE3 types.
In any case, I will ask the Euclid team at ESAC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be clear, my suggestion is to put this into the narrative docs and refer to that from the docstring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(given the overall complexity, I think that's a cleaner place for the users to learn more about the options, also it's more possible to give more context about the options there)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have moved the information of this method to the documentation of the Euclid module. We have included a table at the end of the document and included a reference to it in the method.
docs/esa/euclid/euclid.rst
Outdated
>>> le3_product_list = Euclid.get_scientific_product_list(tile_index=102034414) | ||
>>> print("Found", len(le3_product_list), "results") | ||
Found 12 results | ||
>>> print(le3_product_list) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add the table printout here, or remove the print line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. The documentation was updated with the output of the functions.
docs/esa/euclid/euclid.rst
Outdated
>>> results = euclid.get_scientific_product_list(category='Weak Lensing Products', group='2PCF', product_type='DpdTwoPCFWLShearShear2D') | ||
>>> print("Found", len(le3_product_list), "results") | ||
Found 12 results | ||
>>> print(le3_product_list) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same, either remove the print line, or add an output
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. The documentation was updated with the output of the functions.
@@ -1276,5 +1277,411 @@ def get_datalinks(self, ids, *, linking_parameter='SOURCE_ID', verbose=False): | |||
|
|||
return self.__eucliddata.get_datalinks(ids=ids, linking_parameter=linking_parameter, verbose=verbose) | |||
|
|||
def get_scientific_product_list(self, *, observation_id=None, tile_index=None, category=None, group=None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a small side note from someone not exactly familiar with Euclid data products and use cases, so I could easily be in the wrong:
I'm not sure about the naming, isn't science products returned with get_product_list
, too? maybe swap scientific
to highlevel
or something else? Or do you expect the users mostly using this method to access data?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will ask to the Euclid team at ESAC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eventually, we may revise the names of the methods in the EuclidClass to e.g., homogenize the method names and reduce their length - but that is well beyond the scope of this task. So we would leave the present name of this method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, that sounds good.
docs/esa/euclid/euclid.rst
Outdated
@@ -240,7 +240,30 @@ To get the list of products associated with a given Euclid observation_id or til | |||
EUC_MER_BGSUB-MOSAIC-DES-Z_TILE102018211-83C32F_20241018T143526.104818Z_00.00.fits 1398 102018211 DECAM DECAM_z SCIENCE SKY 57.9990741 -51.5 IMAGE | |||
|
|||
|
|||
The method returns a list of products as an astropy.table. | |||
The method returns a list of products as an astropy.table. It is also possible to search by observation_id, but not by both parameters simultaneously. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method returns a list of products as an astropy.table. It is also possible to search by observation_id, but not by both parameters simultaneously. | |
The method returns a list of products as an `~astropy.table.Table`. It is also possible to search by observation_id, but not by both parameters simultaneously. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation updated.
astroquery/esa/euclid/core.py
Outdated
|
||
Returns | ||
------- | ||
The list of products (astropy.table) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest avoid using 'list' to describe a table as it's slightly misleading
The list of products (astropy.table) | |
The products in an astropy.table.Table |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes committed to the branch.
Dear Astroquery team,
we would like to include a new method in the Euclid project to retrieve scientific LE3 products. This method is similar (in scope) to the following methods:
We have also developed new tests.
cc @esdc-esac-esa-int
jira: EUCLIDMNGT-1275