iteration on has_target from keyword #84
Conversation
|
| return " or ".join(f'{property_name} eq "{s}"' for s in _canonicalize_string(value)) | ||
|
|
||
| q_string = eq_cannonical_string_clause("pds:Identification_Area.pds:title", keyword) | ||
| q_string += f"or {eq_cannonical_string_clause('pds:Alias.pds:alternate_title', keyword)}" |
There was a problem hiding this comment.
Is a leading space before or needed here?
There was a problem hiding this comment.
I think that works without but that will be cleaner with.
| identifiers = [identifiers] | ||
|
|
||
| if len(identifiers) > 0: | ||
| clause = "or ".join([f'ref_lid_target eq "{identifier}"' for identifier in identifiers]) |
There was a problem hiding this comment.
A leading space before the or here might be needed here as well?
There was a problem hiding this comment.
I think that works without but that will be cleaner with.
| @@ -12,7 +12,7 @@ | |||
| # in the `PDS keyword search <https://pds.nasa.gov/datasearch/keyword-search/search.jsp>`_ | |||
| mercury_id = "urn:nasa:pds:context:target:planet.mercury" | |||
| # filter here: | |||
| products = pep.Products(client).has_target(mercury_id).before(date1).observationals() | |||
| products = pep.Products(client).has_target("Mercury").before(date1).observationals() | |||
There was a problem hiding this comment.
Since mercury_id is still defined, maybe we should include an example of both variations of the new has_target convention here.
There was a problem hiding this comment.
Correct, I will fix that.
There was a problem hiding this comment.
Actually I removed it for now since the plan is to have only the keyword search in the quickstart. As we need to enrich the quickstart we can had back new cases as they come in the quickstart in the online doc.
| import unittest | ||
| from datetime import datetime | ||
| from typing import get_args | ||
|
|
||
| import pds.peppi as pep # type: ignore | ||
| from pds.api_client import PdsProduct | ||
|
|
||
| logger = logging.getLogger(__name__) |
There was a problem hiding this comment.
Why was this added? I don't see any logging going on in this test suite now.
There was a problem hiding this comment.
I can remove that, I think I added when I was unable to assert something in the logs, I need to check that this works without that.
🗒️ Summary
⚙️ Test Data and/or Report
Test available in unittest.
♻️ Related Issues
improves #74