asfadmin / Discovery-asf_search

BSD 3-Clause "New" or "Revised" License
131 stars 45 forks source link

[Bug] `search()` only accepts `list[str]` for options marked with `Iterable[str]` #243

Closed scottstanie closed 10 months ago

scottstanie commented 10 months ago

Describe the bug

Most of the arguments of .search are typed such that they accept either a string, or an Iterable of strings. In my example, I was searching for operaBurstID: https://github.com/asfadmin/Discovery-asf_search/blob/master/asf_search/search/search.py#L43

Right now, these options seem to break if you pass any Iterable except a list

To Reproduce

import asf_search as asf
asf.search(operaBurstID=('T078_165486_IW2', 'T078_165485_IW2'), processingLevel='CSLC-STATIC')

This should find two results, but prints:

ERROR:root:HTTP 400: ["The maximum value [ 'T078_165485_IW2')] must be greater than the minimum value [('T078_165486_IW2']"]
ERROR:root:The asf-search module ecountered an error with CMR, and the following message was automatically reported to ASF:

"
Error Message: HTTP 400: ["The maximum value [ 'T078_165485_IW2')] must be greater than the minimum value [('T078_165486_IW2']"]
User Agent: Python/3.10.10; requests/2.28.2; asf_search/6.7.1
Search Options: {
    processingLevel: ['CSLC-STATIC']
    operaBurstID: ["('T078_165486_IW2', 'T078_165485_IW2')"]
}
"If you have any questions email uso@asf.alaska.edu
Out[25]: ASFSearchResults([])

Expected behavior Same as passing a list:

In [27]: asf.search(operaBurstID=['T078_165486_IW2', 'T078_165485_IW2'], processingLevel='CSLC-STATIC')
Out[27]:
ASFSearchResults([<asf_search.ASFProduct.ASFProduct at 0x1530ca4a0>,
                  <asf_search.ASFProduct.ASFProduct at 0x1530cb220>])

Note: Running the parsing which happens inside search:

https://github.com/asfadmin/Discovery-asf_search/blob/master/asf_search/search/search.py#L89-L90C28

ipdb> print(data)
{'processingLevel': 'CSLC-STATIC', 'operaBurstID': ('T078_165486_IW2', 'T078_165485_IW2')}
ipdb> sopts = ASFSearchOptions()
ipdb> sopts.merge_args(**data)
ipdb> print(sopts)
{
    "processingLevel": [
        "CSLC-STATIC"
    ],
    "operaBurstID": [
        "('T078_165486_IW2', 'T078_165485_IW2')"
    ]
}

and I see this is happening in the validators part, since operaBurstID is running the parse_string_list validator https://github.com/asfadmin/Discovery-asf_search/blob/master/asf_search/ASFSearchOptions/validators.py#L96-L99

I'm not sure if anyone wants to put in the effort to convert all the validators from list-parsers into proper Iterator parsers... so the other short-term option would be to change the type annotations on the search functions.

Desktop (please complete the following information):

``` $ conda info active environment : mapping active env location : /Users/staniewi/miniconda3/envs/mapping shell level : 2 user config file : /Users/staniewi/.condarc populated config files : /Users/staniewi/.condarc conda version : 23.9.0 conda-build version : not installed python version : 3.9.18.final.0 virtual packages : __archspec=1=m1 __osx=13.6.2=0 __unix=0=0 base environment : /Users/staniewi/miniconda3 (writable) conda av data dir : /Users/staniewi/miniconda3/etc/conda conda av metadata url : None channel URLs : https://conda.anaconda.org/conda-forge/osx-arm64 https://conda.anaconda.org/conda-forge/noarch https://repo.anaconda.com/pkgs/main/osx-arm64 https://repo.anaconda.com/pkgs/main/noarch https://repo.anaconda.com/pkgs/r/osx-arm64 https://repo.anaconda.com/pkgs/r/noarch package cache : /Users/staniewi/miniconda3/pkgs /Users/staniewi/.conda/pkgs envs directories : /Users/staniewi/miniconda3/envs /Users/staniewi/.conda/envs platform : osx-arm64 user-agent : conda/23.9.0 requests/2.31.0 CPython/3.9.18 Darwin/22.6.0 OSX/13.6.2 UID:GID : 503:20 netrc file : /Users/staniewi/.netrc offline mode : False ```

Additional context Add any other context about the problem here.

SpicyGarlicAlbacoreRoll commented 10 months ago

Thank you for submitting an issue!

asf-search will properly accept and parse iterables with the changes in #251 for the eventual v6.7.3 release!

scottstanie commented 10 months ago

Looks like #251 did fix it! thanks