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.

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

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

Conversation

snbianco
Copy link
Contributor

@snbianco snbianco commented Sep 20, 2024

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 to Observations.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 the Mast.Caom.Products service could filter out duplicates at some point, but I think this is the best solution for now.

@snbianco
Copy link
Contributor Author

snbianco commented Sep 20, 2024

@dr-rodriguez @astrojimig

@snbianco snbianco marked this pull request as ready for review September 20, 2024 12:51
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`")

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"

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

@astrojimig
Copy link

This looks good! Thanks @snbianco !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants