-
-
Notifications
You must be signed in to change notification settings - Fork 396
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
Get list of unique products #3096
Conversation
products = self.get_product_list(observations) | ||
unique_products = self._remove_duplicate_products(products) | ||
if len(unique_products) < len(products): | ||
log.info("To return all products, use `Observations.get_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.
This log message could be slightly more informative if you include the numbers, for example "{len(products) - len(unique_products)} duplicate products removed; To return all products, use Observations.get_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.
Ah never mind, the other log message captures that info! You're way ahead of me :)
This looks good! Thanks @snbianco ! |
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.
Looks good, thanks! I would suggest a follow-up docs cleanup PR that would make the links better.
""" | ||
Given a "Product Group Id" (column name obsid), returns a list of associated data products with | ||
unique dataURIs. Note that obsid is NOT the same as obs_id, and inputting obs_id values will result in | ||
an error. See column documentation `here <https://masttest.stsci.edu/api/v0/_productsfields.html>`__. |
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.
Nitpicking, but it would be nice to avoid (and also clean up) these types of here
links. Some reading in the topics: https://heyoka.medium.com/dont-use-click-here-f32f445d1021
We have a bunch of these already, mostly in the mast module, so I don't hold merging up, but I would really encourage a cleanup.
Added implementation, a test, and documentation for a new function (
Observations.get_unique_product_list
). Given observations, the function returns a list of associated data products, each with a unique data URI. This is essentially a wrapper toObservations.get_product_list
that removes duplicate rows before returning.Because
get_product_list
is a query method, it should be async and return a response that is then parsed into a Table. Because we need the actual result Table to remove the duplicates, I wasn't able to add this functionality as a parameter to the original function. It's possible that theMast.Caom.Products
service could filter out duplicates at some point, but I think this is the best solution for now.