OSGeo / grass-addons

GRASS GIS Addons Repository
https://grass.osgeo.org/grass-stable/manuals/addons/
GNU General Public License v2.0
98 stars 148 forks source link

i.eodag: add operators to queryables and allow multiple values #1142

Closed HamedElgizery closed 1 month ago

HamedElgizery commented 1 month ago

This PR fixes the current faulty query option, and add the ability to use operators (eq, lt, gt, le, etc...). It also allows the query option to have multiple queryables (emulating the AND relation), and allows each queryable to have multiple value (emulating the OR relation).

Querying section is also added to the manual, with examples on how to use the query option. Multiple use cases for i.eodag is also added to the manual.

veroandreo commented 1 month ago

@HamedElgizery I extensively modified the manual page to demonstrate the usual writing style. Please, have a look to check that I got things right (i.e., search products intersecting the computational region should be the default). I removed the -e flag in one example, as that flag is no longer there. Also, I believe some other options might be explained in the description section.

And, as far as I recall from eodag itself, intersects is the default, so area_relation should have answer:Intersects as default.

I leave the code review to @ninsbl and @lucadelu :)

lucadelu commented 1 month ago

I looked at the code and I don't see big problems. I tested the pull request and most of the command I tried it works.

I have a problem trying to query using the id, when I use the id parameter it return an empty result. It could be a eodag problem and not related to the GRASS implementation...

GRASS utm32n/lucadelu:i.eodag > python i.eodag.py -l producttype=S2_MSI_L2A query="start=2022-05-25T00:00:00,end=2022-06-01T23:59:00"
WARNING: Experimental Version...
WARNING: This module is still under development, and its behaviour is not
         guaranteed to be reliable
3 scenes(s) found.
S2B_MSIL2A_20220526T101559_N0400_R065_T32TPS_20220526T134056 2022-05-26T10:15:59 50% S2MSI2A
S2A_MSIL2A_20220531T101611_N0400_R065_T32TPS_20220531T163910 2022-05-31T10:16:11 59% S2MSI2A
S2A_MSIL2A_20220528T100601_N0400_R022_T32TPS_20220528T162819 2022-05-28T10:06:01 70% S2MSI2A
GRASS utm32n/lucadelu:i.eodag > python i.eodag.py -l producttype=S2_MSI_L2A query="id=S2B_MSIL2A_20220526T101559_N0400_R065_T32TPS_20220526T134056\eq"
WARNING: Experimental Version...
WARNING: This module is still under development, and its behaviour is not
         guaranteed to be reliable
0 scenes(s) found.
neteler commented 1 month ago

I made an initial test, great work!

A few observations:

Error to be caught when input vector map not present (just try with a non-existing one):

GRASS latlong_wgs84/testarea:~ > i.eodag -l start=2017-10-21 end=2017-10-21 producttype=S2_MSI_L2A provider=cop_dataspace map=non_existing_bbox_map
WARNING: Experimental Version...
WARNING: This module is still under development, and its behaviour is not
         guaranteed to be reliable
ERROR: Vector map <non_existing_bbox_map> not found
Traceback (most recent call last):
  File "/home/mneteler/software/grass84/dist.x86_64-pc-linux-gnu/scripts/i.eodag", line 1144, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/mneteler/software/grass84/dist.x86_64-pc-linux-gnu/scripts/i.eodag", line 1057, in main
    geometry = get_aoi(options["map"])
               ^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/mneteler/software/grass84/dist.x86_64-pc-linux-gnu/scripts/i.eodag", line 296, in get_aoi
    if gs.vector_info_topo(vector)["areas"] <= 0:
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/mneteler/software/grass84/dist.x86_64-pc-linux-gnu/etc/python/grass/script/vector.py", line 182, in vector_info_topo
    s = read_command("v.info", flags="t", layer=layer, map=map, env=env)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/mneteler/software/grass84/dist.x86_64-pc-linux-gnu/etc/python/grass/script/core.py", line 554, in read_command
    return handle_errors(returncode, stdout, args, kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/mneteler/software/grass84/dist.x86_64-pc-linux-gnu/etc/python/grass/script/core.py", line 366, in handle_errors
    raise CalledModuleError(module=module, code=code, returncode=returncode)
grass.exceptions.CalledModuleError: Module run `v.info -t layer=1 map=non_existing_bbox_map` ended with an error.
The subprocess ended with a non-zero return code: 1. See errors above the traceback or in the error output.

And it would be good to have a default cloud value defined and visible to the user as it is unclear from the description, to which threshold cloud is set per default (could be 20% or whatever):

clouds   Maximum cloud cover percentage for scene [0, 100]
default: 20

And: which is the default output directory?

output Name for output directory where to store downloaded scenes data

Indeed I found the downloaded S2 data in $HOME/tmp/grass8-mneteler-619940/ (Linux) which is unexpected for me. Say, r.out.gdal etc. use the current directory as the default which is probably more intuitive.

Thanks again for your work, my first i.eodag downloads are running :-)

HamedElgizery commented 1 month ago

@HamedElgizery I extensively modified the manual page to demonstrate the usual writing style. Please, have a look to check that I got things right (i.e., search products intersecting the computational region should be the default). I removed the -e flag in one example, as that flag is no longer there. Also, I believe some other options might be explained in the description section.

Thanks, @veroandreo. It is more consistent and clear now. I have gone through it and everything looks right. I only added some <p> tags to separate the subtitles from the content in this commit dfb364a.

And, as far as I recall from eodag itself, intersects is the default, so area_relation should have answer:Intersects as default.

Just added that.

HamedElgizery commented 1 month ago

I looked at the code and I don't see big problems. I tested the pull request and most of the command I tried it works.

I have a problem trying to query using the id, when I use the id parameter it return an empty result. It could be a eodag problem and not related to the GRASS implementation...

GRASS utm32n/lucadelu:i.eodag > python i.eodag.py -l producttype=S2_MSI_L2A query="start=2022-05-25T00:00:00,end=2022-06-01T23:59:00"
WARNING: Experimental Version...
WARNING: This module is still under development, and its behaviour is not
         guaranteed to be reliable
3 scenes(s) found.
S2B_MSIL2A_20220526T101559_N0400_R065_T32TPS_20220526T134056 2022-05-26T10:15:59 50% S2MSI2A
S2A_MSIL2A_20220531T101611_N0400_R065_T32TPS_20220531T163910 2022-05-31T10:16:11 59% S2MSI2A
S2A_MSIL2A_20220528T100601_N0400_R022_T32TPS_20220528T162819 2022-05-28T10:06:01 70% S2MSI2A
GRASS utm32n/lucadelu:i.eodag > python i.eodag.py -l producttype=S2_MSI_L2A query="id=S2B_MSIL2A_20220526T101559_N0400_R065_T32TPS_20220526T134056\eq"
WARNING: Experimental Version...
WARNING: This module is still under development, and its behaviour is not
         guaranteed to be reliable
0 scenes(s) found.

To filter by the id you need to use the queryable title instead. So giving that you are using the same AOI, the following should work:

python i.eodag.py -l producttype=S2_MSI_L2A query="title=S2B_MSIL2A_20220526T101559_N0400_R065_T32TPS_20220526T134056\eq"

Or... You can use search by ID from scratch as:

python i.eodag.py -l producttype=S2_MSI_L2A id=S2B_MSIL2A_20220526T101559_N0400_R065_T32TPS_20220526T134056
veroandreo commented 1 month ago

I will merge this one once tests finish. The remaining "questions" were moved here: https://github.com/HamedElgizery/grass-addons/issues/17

HamedElgizery commented 1 month ago

And, as far as I recall from eodag itself, intersects is the default, so area_relation should have answer:Intersects as default.

Just added that.

I reverted this change (setting area_relation to Intersects as a default value), and now it has no default value. The reason is that the default value for area_relation makes i.eodag do unintended filtering for products passed from a GeoJSON file. If filtering of products from a GeoJSON file with respect to the area_relation is needed, then area_relation needs to be specified explicitly.

It was reverted here 44150b7569cbd8381b49660b97b512de73358581.