From 8945f64dc26b9acf8eac03848a3e326d2297b327 Mon Sep 17 00:00:00 2001 From: Scott Collins Date: Wed, 8 Jan 2025 13:46:17 -0800 Subject: [PATCH 1/9] Added string representation methods to ResultSet and QueryBuilder --- src/pds/peppi/query_builder.py | 13 +++++-------- src/pds/peppi/result_set.py | 4 ++++ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/pds/peppi/query_builder.py b/src/pds/peppi/query_builder.py index 2b3833e..7d6c009 100644 --- a/src/pds/peppi/query_builder.py +++ b/src/pds/peppi/query_builder.py @@ -18,17 +18,14 @@ class QueryBuilder: """QueryBuilder provides method to elaborate complex PDS queries.""" def __init__(self): - """Creates a new instance of the QueryBuilder class. - - Parameters - ---------- - client: PDSRegistryClient - The client object used to interact with the PDS Registry API. - - """ + """Creates a new instance of the QueryBuilder class.""" self._q_string = "" self._fields: list[str] = [] + def __str__(self): + """Returns a formatted string representation of the current query.""" + return "\n and".join(self._q_string.split("and")) + def __add_clause(self, clause): """Adds the provided clause to the query string to use on the next fetch of products from the Registry API. diff --git a/src/pds/peppi/result_set.py b/src/pds/peppi/result_set.py index 98db408..0be0eee 100644 --- a/src/pds/peppi/result_set.py +++ b/src/pds/peppi/result_set.py @@ -28,6 +28,10 @@ def __init__(self, client: PDSRegistryClient): self._page_counter = None self._expected_pages = None + def __str__(self): + """Returns a string representation of the ResultSet, including the current query string.""" + return f"Current {self.__class__.__name__} query:\n{super().__str__()}" + def _init_new_page(self): """Queries the PDS API for the next page of results. From 14a14e35916277acd3972ef14d76dd6b8613f014 Mon Sep 17 00:00:00 2001 From: Scott Collins Date: Wed, 8 Jan 2025 13:49:09 -0800 Subject: [PATCH 2/9] Changed QueryBuilder.__add_clause to be "protected" instead of "private" so it can be accessed by inheritors --- src/pds/peppi/query_builder.py | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/pds/peppi/query_builder.py b/src/pds/peppi/query_builder.py index 7d6c009..919e71e 100644 --- a/src/pds/peppi/query_builder.py +++ b/src/pds/peppi/query_builder.py @@ -26,7 +26,7 @@ def __str__(self): """Returns a formatted string representation of the current query.""" return "\n and".join(self._q_string.split("and")) - def __add_clause(self, clause): + def _add_clause(self, clause): """Adds the provided clause to the query string to use on the next fetch of products from the Registry API. Repeated calls to this method results in a joining with any previously @@ -85,7 +85,7 @@ def has_target(self, identifier: str): """ clause = f'ref_lid_target eq "{identifier}"' - self.__add_clause(clause) + self._add_clause(clause) return self def has_investigation(self, identifier: str): @@ -102,7 +102,7 @@ def has_investigation(self, identifier: str): """ clause = f'ref_lid_investigation eq "{identifier}"' - self.__add_clause(clause) + self._add_clause(clause) return self def before(self, dt: datetime): @@ -120,7 +120,7 @@ def before(self, dt: datetime): """ iso8601_datetime = dt.isoformat().replace("+00:00", "Z") clause = f'pds:Time_Coordinates.pds:start_date_time le "{iso8601_datetime}"' - self.__add_clause(clause) + self._add_clause(clause) return self def after(self, dt: datetime): @@ -138,7 +138,7 @@ def after(self, dt: datetime): """ iso8601_datetime = dt.isoformat().replace("+00:00", "Z") clause = f'pds:Time_Coordinates.pds:stop_date_time ge "{iso8601_datetime}"' - self.__add_clause(clause) + self._add_clause(clause) return self def of_collection(self, identifier: str): @@ -155,7 +155,7 @@ def of_collection(self, identifier: str): """ clause = f'ops:Provenance.ops:parent_collection_identifier eq "{identifier}"' - self.__add_clause(clause) + self._add_clause(clause) return self def observationals(self): @@ -167,7 +167,7 @@ def observationals(self): """ clause = 'product_class eq "Product_Observational"' - self.__add_clause(clause) + self._add_clause(clause) return self def collections(self, collection_type: Optional[str] = None): @@ -185,11 +185,11 @@ def collections(self, collection_type: Optional[str] = None): """ clause = 'product_class eq "Product_Collection"' - self.__add_clause(clause) + self._add_clause(clause) if collection_type: clause = f'pds:Collection.pds:collection_type eq "{collection_type}"' - self.__add_clause(clause) + self._add_clause(clause) return self @@ -202,7 +202,7 @@ def bundles(self): """ clause = 'product_class eq "Product_Bundle"' - self.__add_clause(clause) + self._add_clause(clause) return self def has_instrument(self, identifier: str): @@ -219,7 +219,7 @@ def has_instrument(self, identifier: str): """ clause = f'ref_lid_instrument eq "{identifier}"' - self.__add_clause(clause) + self._add_clause(clause) return self def has_instrument_host(self, identifier: str): @@ -236,7 +236,7 @@ def has_instrument_host(self, identifier: str): """ clause = f'ref_lid_instrument_host eq "{identifier}"' - self.__add_clause(clause) + self._add_clause(clause) return self def has_processing_level(self, processing_level: PROCESSING_LEVELS = "raw"): @@ -254,7 +254,7 @@ def has_processing_level(self, processing_level: PROCESSING_LEVELS = "raw"): """ clause = f'pds:Primary_Result_Summary.pds:processing_level eq "{processing_level.title()}"' - self.__add_clause(clause) + self._add_clause(clause) return self def get(self, identifier: str): @@ -270,7 +270,7 @@ def get(self, identifier: str): This Products instance with the "LIDVID identifier" filter applied. """ - self.__add_clause(f'lidvid like "{identifier}"') + self._add_clause(f'lidvid like "{identifier}"') return self def fields(self, fields: list): @@ -290,5 +290,5 @@ def filter(self, clause: str): ------- This Products instance with the provided filtering clause applied. """ - self.__add_clause(clause) + self._add_clause(clause) return self From afa431884808c9855737f6b72476c2791ba55889 Mon Sep 17 00:00:00 2001 From: Scott Collins Date: Wed, 8 Jan 2025 13:49:49 -0800 Subject: [PATCH 3/9] Added stub implementations for QueryBuilder.within_range() and QueryBuilder.within_bbox() --- src/pds/peppi/query_builder.py | 46 ++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src/pds/peppi/query_builder.py b/src/pds/peppi/query_builder.py index 919e71e..e71f661 100644 --- a/src/pds/peppi/query_builder.py +++ b/src/pds/peppi/query_builder.py @@ -257,6 +257,52 @@ def has_processing_level(self, processing_level: PROCESSING_LEVELS = "raw"): self._add_clause(clause) return self + def within_range(self, range_in_km: float): + """Adds a query clause selecting products within the provided range value. + + Notes + ----- + This method should be implemented by product-specific inheritors that + support the notion of range to a given target. + + Parameters + ---------- + range_in_km : float + The range in kilometers to use with the query. + + Raises + ------ + NotImplementedError + + """ + raise NotImplementedError("within_range is not available for base QueryBuilder") + + def within_bbox(self, lat_min: float, lat_max: float, lon_min: float, lon_max: float): + """Adds a query clause selecting products which fall within the bounds of the provided bounding box. + + Notes + ----- + This method should be implemented by product-specific inheritors that + support the notion of bounding box to filter results by. + + Parameters + ---------- + lat_min : float + Minimum latitude boundary. + lat_max : float + Maximum latitude boundary. + lon_min : float + Minimum longitude boundary. + lon_max : float + Maximum longitude boundary. + + Raises + ------ + NotImplementedError + + """ + raise NotImplementedError("within_bbox is not available for base QueryBuilder") + def get(self, identifier: str): """Adds a query clause selecting the product with a LIDVID matching the provided value. From 3aadf9553d012add3b5d4b1b359edba24f46d09a Mon Sep 17 00:00:00 2001 From: Scott Collins Date: Wed, 8 Jan 2025 13:50:12 -0800 Subject: [PATCH 4/9] Initial addition of the orex package This package contains an specialized query builder implementation that supports within_range and within_bbox calls for use with Osiris Rex products. --- src/pds/peppi/__init__.py | 6 +-- src/pds/peppi/orex/__init__.py | 2 + src/pds/peppi/orex/products.py | 18 +++++++ src/pds/peppi/orex/result_set.py | 87 ++++++++++++++++++++++++++++++++ 4 files changed, 108 insertions(+), 5 deletions(-) create mode 100644 src/pds/peppi/orex/__init__.py create mode 100644 src/pds/peppi/orex/products.py create mode 100644 src/pds/peppi/orex/result_set.py diff --git a/src/pds/peppi/__init__.py b/src/pds/peppi/__init__.py index 1c8f20d..5394587 100644 --- a/src/pds/peppi/__init__.py +++ b/src/pds/peppi/__init__.py @@ -1,9 +1,5 @@ # -*- coding: utf-8 -*- """PDS peppi.""" from .client import PDSRegistryClient # noqa +from .orex import OrexProducts # noqa from .products import Products # noqa - -# For future consideration: -# -# - Other metadata (__docformat__, __copyright__, etc.) -# - N̶a̶m̶e̶s̶p̶a̶c̶e̶ ̶p̶a̶c̶k̶a̶g̶e̶s̶ we got this diff --git a/src/pds/peppi/orex/__init__.py b/src/pds/peppi/orex/__init__.py new file mode 100644 index 0000000..4175f70 --- /dev/null +++ b/src/pds/peppi/orex/__init__.py @@ -0,0 +1,2 @@ +"""Osiris Rex (OREX) tailored package for use with Peppi.""" +from .products import OrexProducts # noqa diff --git a/src/pds/peppi/orex/products.py b/src/pds/peppi/orex/products.py new file mode 100644 index 0000000..e094c6d --- /dev/null +++ b/src/pds/peppi/orex/products.py @@ -0,0 +1,18 @@ +"""Main class of the Osiris Rex (OREX) Peppi package.""" +from pds.peppi.client import PDSRegistryClient +from pds.peppi.orex.result_set import OrexResultSet + + +class OrexProducts(OrexResultSet): + """Specialized Products class used to query specfically for Osiris Rex (OREX) products.""" + + def __init__(self, client: PDSRegistryClient): + """Creates a new instance of OrexProducts. + + Parameters + ---------- + client : PDSRegistryClient + Client defining the connection with the PDS Search API. + + """ + super().__init__(client) diff --git a/src/pds/peppi/orex/result_set.py b/src/pds/peppi/orex/result_set.py new file mode 100644 index 0000000..0b24fbe --- /dev/null +++ b/src/pds/peppi/orex/result_set.py @@ -0,0 +1,87 @@ +"""Module containing the Osiris Rex (OREX) tailored ResultSet class.""" +from pds.peppi.client import PDSRegistryClient +from pds.peppi.result_set import ResultSet + + +class OrexResultSet(ResultSet): + """Inherits the functionality of the ResultSet class, but adds implementations for stubs in QueryBuilder.""" + + orex_instrument_lidvid = "urn:nasa:pds:context:instrument:ovirs.orex" + + def __init__(self, client: PDSRegistryClient): + """Creates a new instance of OrexResultSet. + + Parameters + ---------- + client : PDSRegistryClient + Client defining the connection with the PDS Search API. + + """ + super().__init__(client) + + # By default, all query results are filtered to just those applicable to + # the OREX instrument + self._q_string = f'ref_lid_instrument eq "{self.orex_instrument_lidvid}"' + + def has_instrument(self, identifier: str): + """Adds a query clause selecting products having an instrument matching the provided identifier. + + Notes + ----- + For OrexResultsSet, this method is not impemented since the instrument + is implicitly always fixed to Osiris Rex. + + Parameters + ---------- + identifier : str + Identifier (LIDVID) of the instrument. + + Raises + ------ + NotImplementedError + + """ + raise NotImplementedError(f"Cannot specify an additional instrument on {self.__class__.__name__}") + + def within_range(self, range_in_km: float): + """Adds a query clause selecting products within the provided range value. + + Parameters + ---------- + range_in_km : float + The range in kilometers to use with the query. + + Returns + ------- + This OrexResultSet instance with the "within range" filter applied. + + """ + self._add_clause(f"orex:Spatial.orex:target_range le {range_in_km}") + + return self + + def within_bbox(self, lat_min: float, lat_max: float, lon_min: float, lon_max: float): + """Adds a query clause selecting products which fall within the bounds of the provided bounding box. + + Parameters + ---------- + lat_min : float + Minimum latitude boundary. + lat_max : float + Maximum latitude boundary. + lon_min : float + Minimum longitude boundary. + lon_max : float + Maximum longitude boundary. + + Returns + ------- + This OrexResultSet instance with the "within bounding box" filter applied. + + """ + self._add_clause(f"orex:Spatial.orex:latitude ge {lat_min}") + self._add_clause(f"orex:Spatial.orex:latitude le {lat_max}") + self._add_clause(f"orex:Spatial.orex:longitude ge {lon_min}") + self._add_clause(f"orex:Spatial.orex:longitude le {lon_max}") + + return self From ca836f39b4b5cd5f7f9a5ff02a5ad8a7a9eacd2c Mon Sep 17 00:00:00 2001 From: Scott Collins Date: Wed, 8 Jan 2025 13:53:36 -0800 Subject: [PATCH 5/9] Removed demo_candidate modules, as we have settled on candidate B --- tests/pds/peppi/demo_candidates_A.py | 24 ------------------------ tests/pds/peppi/demo_candidates_B.py | 21 --------------------- tests/pds/peppi/demo_candidates_C.py | 21 --------------------- 3 files changed, 66 deletions(-) delete mode 100644 tests/pds/peppi/demo_candidates_A.py delete mode 100644 tests/pds/peppi/demo_candidates_B.py delete mode 100644 tests/pds/peppi/demo_candidates_C.py diff --git a/tests/pds/peppi/demo_candidates_A.py b/tests/pds/peppi/demo_candidates_A.py deleted file mode 100644 index bc040bf..0000000 --- a/tests/pds/peppi/demo_candidates_A.py +++ /dev/null @@ -1,24 +0,0 @@ -import pds.peppi as pep - -# ** option A ** -# a QueryBuilder has an explicit evaluation method called `crack` -# which triggers the query to be evaluated and returns a ResultSet object -# PRO: -# - explicit evaluation of the result -# - 'crack' could be kind of fun for peppi, but we could be more traditional with 'open' or 'execute' -# CON: -# - crack or open might sound useless in the client code, or unclear what it's useful for -lidvid = "urn:nasa:pds:context:target:asteroid.65803_didymos" -query = pep.QueryBuilder().has_target(lidvid).observationals() - -client = pep.PDSRegistryClient() -products = query.crack(client) - -# do something sequentially -for i, p in enumerate(products): - print(p.id) - if i > 10: - break - -# get all the results at once in a new object, here pandas dataframe -df = query.crack(client).as_dataframe(max_rows=10) diff --git a/tests/pds/peppi/demo_candidates_B.py b/tests/pds/peppi/demo_candidates_B.py deleted file mode 100644 index 94f7eb6..0000000 --- a/tests/pds/peppi/demo_candidates_B.py +++ /dev/null @@ -1,21 +0,0 @@ -import pds.peppi as pep - -# ** option B ** -# Same as before, but under the hood the Products inherit ResultSet and QueryBuilder -# to separate concerns in classes and simplify the code -# PRO: user code is short -# CON: blurred boundaries between Query and Result -client = pep.PDSRegistryClient() - -lidvid = "urn:nasa:pds:context:target:asteroid.65803_didymos" -products = pep.Products(client) -products = products.has_target(lidvid).observationals() - -# do something sequentially -for i, p in enumerate(products): - print(p.id) - if i > 10: - break - -# get all the results at once in a new object, here pandas dataframe -df = products.as_dataframe(max_rows=10) diff --git a/tests/pds/peppi/demo_candidates_C.py b/tests/pds/peppi/demo_candidates_C.py deleted file mode 100644 index ec66a34..0000000 --- a/tests/pds/peppi/demo_candidates_C.py +++ /dev/null @@ -1,21 +0,0 @@ -import pds.peppi as pep - -# ** option C ** -# Products result is instantiated from both the connexion to the API and the query -# PRO: that sounds the most simple and readable implementation for the user -# CON: we loose the elegance of the Query being evaluated smoothly into a ResultSet - -client = pep.PDSRegistryClient() - -lidvid = "urn:nasa:pds:context:target:asteroid.65803_didymos" -query = pep.QueryBuilder().has_target(lidvid).observationals() -products = pep.Products(client, query) - -# do something sequentially -for i, p in enumerate(products): - print(p.id) - if i > 10: - break - -# get all the results at once in a new object, here pandas dataframe -df = products.as_dataframe(max_rows=10) From 88e638345b64ca84290fed7a46ae0a4b2f04fd17 Mon Sep 17 00:00:00 2001 From: Scott Collins Date: Wed, 8 Jan 2025 15:12:30 -0800 Subject: [PATCH 6/9] Renamed test_client.py to test_products.py --- tests/pds/peppi/{test_client.py => test_products.py} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename tests/pds/peppi/{test_client.py => test_products.py} (99%) diff --git a/tests/pds/peppi/test_client.py b/tests/pds/peppi/test_products.py similarity index 99% rename from tests/pds/peppi/test_client.py rename to tests/pds/peppi/test_products.py index 629f3fb..6bbe6c0 100644 --- a/tests/pds/peppi/test_client.py +++ b/tests/pds/peppi/test_products.py @@ -6,7 +6,7 @@ from pds.api_client import PdsProduct -class ClientTestCase(unittest.TestCase): +class ProductsTestCase(unittest.TestCase): MAX_ITERATIONS = 1000 """Maximum number of iterations over query results to perform before ending test.""" From 19b472cf641dc793fad129cd3a9aabbab0d56812 Mon Sep 17 00:00:00 2001 From: Scott Collins Date: Wed, 8 Jan 2025 15:12:52 -0800 Subject: [PATCH 7/9] Added unit tests for the OrexProducts class --- tests/pds/peppi/test_orex_products.py | 47 +++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 tests/pds/peppi/test_orex_products.py diff --git a/tests/pds/peppi/test_orex_products.py b/tests/pds/peppi/test_orex_products.py new file mode 100644 index 0000000..2a99424 --- /dev/null +++ b/tests/pds/peppi/test_orex_products.py @@ -0,0 +1,47 @@ +import unittest + +import pds.peppi as pep + + +class OrexProductsTestCase(unittest.TestCase): + MAX_ITERATIONS = 1000 + """Maximum number of iterations over query results to perform before ending test.""" + + def setUp(self) -> None: + self.client = pep.PDSRegistryClient() + self.products = pep.OrexProducts(self.client) + + def test_has_instrument(self): + # Check that an instance of OrexProducts is initialized with a has_instrument filter + self.assertIn('ref_lid_instrument eq "urn:nasa:pds:context:instrument:ovirs.orex"', self.products._q_string) + + # Check that an attempt to change the instrument results in an exception + with self.assertRaises(NotImplementedError): + self.products.has_instrument("urn:nasa:pds:context:instrument:cls.farir_beamline") + + def test_within_range(self): + n = 0 + for p in self.products.within_range(100.0): + n += 1 + + assert "orex:Spatial.orex:target_range" in p.properties + assert len(p.properties["orex:Spatial.orex:target_range"]) >= 1 + assert all(float(target_range) <= 100.0 for target_range in p.properties["orex:Spatial.orex:target_range"]) + + if n > self.MAX_ITERATIONS: + break + + def test_within_bbox(self): + n = 0 + for p in self.products.within_bbox(9.0, 15.0, 21.0, 27.0): + n += 1 + + assert "orex:Spatial.orex:latitude" in p.properties + assert "orex:Spatial.orex:longitude" in p.properties + assert len(p.properties["orex:Spatial.orex:latitude"]) >= 1 + assert len(p.properties["orex:Spatial.orex:longitude"]) >= 1 + assert all(9.0 <= float(latitude) <= 15.0 for latitude in p.properties["orex:Spatial.orex:latitude"]) + assert all(21.0 <= float(longitude) <= 27.0 for longitude in p.properties["orex:Spatial.orex:longitude"]) + + if n > self.MAX_ITERATIONS: + break From 692dc5eb4234c2c293ceffd659c567696370d998 Mon Sep 17 00:00:00 2001 From: Scott Collins Date: Wed, 8 Jan 2025 15:21:16 -0800 Subject: [PATCH 8/9] Whitespace cleanup on codeql-analysis.yml --- .github/workflows/codeql-analysis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 2bff0bb..a807440 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -114,8 +114,8 @@ jobs: uses: djdefi/cloc-action@6 with: options: --report-file=cloc.md - - + + - name: Upload SLOC uses: actions/upload-artifact@v4 From f32eed7956b494a5cee59d88ecdb95157d3cb31a Mon Sep 17 00:00:00 2001 From: Scott Collins Date: Wed, 8 Jan 2025 15:21:37 -0800 Subject: [PATCH 9/9] Removed mypy from pre-commit hooks --- .pre-commit-config.yaml | 8 -------- 1 file changed, 8 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f7f1521..97cd51e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -18,14 +18,6 @@ repos: - id: reorder-python-imports files: ^src/|tests/ -- repo: local - hooks: - - id: mypy - name: mypy - entry: mypy src - language: system - pass_filenames: false - - repo: local hooks: - id: black