aria-tools / ARIA-tools

Tools for exploiting ARIA standard products
Apache License 2.0
98 stars 35 forks source link

[BUG] ASF Search library now expects WKT string to geo_search #310

Closed scottstanie closed 2 years ago

scottstanie commented 2 years ago

Describe the bug A clear and concise description of what the bug is.

Following an example notbook, I ran ariaDownload.py -b '37.25 38.1 -122.6 -121.75' --track 42 and got this error:

(mapping) [staniewi@aurora sanAnd]$ ariaDownload.py -b '37.25 38.1 -122.6 -121.75' --track 42
ARIA-tools Version: 1.1.3
Traceback (most recent call last):
  File "/u/aurora-r0/staniewi/miniconda3/envs/mapping/bin/ariaDownload.py", line 7, in <module>
    exec(compile(f.read(), __file__, 'exec'))
  File "/u/aurora-r0/staniewi/repos/ARIA-tools/tools/bin/ariaDownload.py", line 290, in <module>
    Downloader(inps)()
  File "/u/aurora-r0/staniewi/repos/ARIA-tools/tools/bin/ariaDownload.py", line 189, in __call__
    scenes     = self.query_asf()
  File "/u/aurora-r0/staniewi/repos/ARIA-tools/tools/bin/ariaDownload.py", line 277, in query_asf
    scenes = asf.geo_search(**dct_kw)
  File "/u/aurora-r0/staniewi/repos/ARIA-tools/Discovery-asf_search/asf_search/search/geo_search.py", line 60, in geo_search
    setattr(opts, p, data[p])
  File "/u/aurora-r0/staniewi/repos/ARIA-tools/Discovery-asf_search/asf_search/ASFSearchOptions/ASFSearchOptions.py", line 37, in __setattr__
    super().__setattr__(key, validate(key, value))
  File "/u/aurora-r0/staniewi/repos/ARIA-tools/Discovery-asf_search/asf_search/ASFSearchOptions/validator_map.py", line 19, in validate
    return validator_map[key](value)
  File "/u/aurora-r0/staniewi/repos/ARIA-tools/Discovery-asf_search/asf_search/ASFSearchOptions/validators.py", line 184, in parse_wkt
    value = wkt.loads(value)
  File "/u/aurora-r0/staniewi/miniconda3/envs/mapping/lib/python3.10/site-packages/shapely/wkt.py", line 22, in loads
    return geos.WKTReader(geos.lgeos).read(data)
  File "/u/aurora-r0/staniewi/miniconda3/envs/mapping/lib/python3.10/site-packages/shapely/geos.py", line 327, in read
    raise TypeError("Only str is accepted.")
TypeError: Only str is accepted.

The issue is return geos.WKTReader(geos.lgeos).read(data) is getting passed a shapely Polygon (from ARIA Tools). I'm not sure if ASF search used to accept/expect that, but now they don't check for that type of input in their validators.py.

Desktop (please complete the following information):

(mapping) [staniewi@aurora ARIA-tools]$ git show --summary
commit f7a634062b3d30dd56bb405bb8b63e407e531e17
Merge: a56a8e5 092d052
Author: Simran S Sangha <sssangha@ucla.edu>
Date:   Mon Jun 13 18:33:10 2022 -0700

    Merge pull request #306 from aria-tools/notebook_tweaks

    Notebook tweaks
bbuzz31 commented 2 years ago

@scottstanie thanks for finding and the fix

bbuzz31 commented 2 years ago

@jhkennedy @SpicyGarlicAlbacoreRoll would it be possible to put an ariaDownload run into the testing suite of ASF_search? there are no non-standard dependencies in ariaDownload except for a logging and small function in ariaTools

jhkennedy commented 2 years ago

@bbuzz31 asf_search just a a major release, meaning they indicated they were possibly making incompatible API changes. Fix is either pinning to the previous major version or updating behavior (typically both; quick fix and then long term fix)

In general it's not the responsibility of a library/package to test behavior of down stream usage. It'd be better to put a unit test in ARIA-tools that would catch an issue like this and regularly run the tests (if development/prs aren't often enough to quickly catch stuff)