i.landsat.download: change API from landsatxplore to EODAG using i.eodag

[HamedElgizery]

This PR will change the API used in i.landsat.download from landsatxplore to EODAG using the i.eodag addon.

[HamedElgizery]

In order to test this branch, you need to update your i.eodag module with the following PR https://github.com/OSGeo/grass-addons/pull/1145, because there is an update in i.eodag (adding regex pattern option), on which searching/downloading with USGS depends.

[HamedElgizery]

However, download with id yields the following in all my attempts (also which is the id to use? maybe when printing we could indicate that?):

Bug fixed in the last commit 82096e8

You need to use the expanded ID (EODAG calls it the title), so this one: LC08_L2SP_193029_20230110_02_T1

[veroandreo]

However, download with id yields the following in all my attempts (also which is the id to use? maybe when printing we could indicate that?):

Bug fixed in the last commit 82096e8

You need to use the expanded ID (EODAG calls it the title), so this one: LC08_L2SP_193029_20230110_02_T1

Different error now:

i.landsat.download id=LC08_L2SP_193029_20230110_02_T1
WARNING: Experimental Version...
WARNING: This module is still under development, and its behaviour is not
         guaranteed to be reliable
Found 1 distinct ID(s).
LC08_L2SP_193029_20230110_02_T1
Traceback (most recent call last):
  File "/home/vandreo/.grass8/addons/scripts/i.eodag", line 1152, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/vandreo/.grass8/addons/scripts/i.eodag", line 1055, in main
    search_result = search_by_ids(ids_set)
                    ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vandreo/.grass8/addons/scripts/i.eodag", line 359, in search_by_ids
    product, count = dag.search(id=query_id, provider=options["provider"] or None)
    ^^^^^^^^^^^^^^
ValueError: not enough values to unpack (expected 2, got 1)
Traceback (most recent call last):
  File "/home/vandreo/software/grass-addons/src/imagery/i.landsat/i.landsat.download/i.landsat.download.py", line 415, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/vandreo/software/grass-addons/src/imagery/i.landsat/i.landsat.download/i.landsat.download.py", line 361, in main
    gs.run_command(
  File "/home/vandreo/software/grass-dev/dist.x86_64-pc-linux-gnu/etc/python/grass/script/core.py", line 485, in run_command
    return handle_errors(returncode, result=None, args=args, kwargs=kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vandreo/software/grass-dev/dist.x86_64-pc-linux-gnu/etc/python/grass/script/core.py", line 364, in handle_errors
    raise CalledModuleError(module=module, code=code, returncode=returncode)
grass.exceptions.CalledModuleError: Module run `i.eodag id=LC08_L2SP_193029_20230110_02_T1 output=/tmp provider=planetary_computer` ended with an error.
The subprocess ended with a non-zero return code: 1. See errors above the traceback or in the error output.

[HamedElgizery]

Different error now:

It was a problem in i.eodag, because of EODAG beta version 3.0.0b1... They added a new parameter count to their search method. The fix is in this PR https://github.com/OSGeo/grass-addons/pull/1145.

[ninsbl]

BTW: Do you use pre-commit hooks? If not, I recommend installing them with python3 -m pip install pre-commit. That way your code is quality checked on each commit...

[HamedElgizery]

BTW: Do you use pre-commit hooks? If not, I recommend installing them with python3 -m pip install pre-commit. That way your code is quality checked on each commit...

Thanks noted. I switched machines and forgot to use the pre-commit on the new one.

[HamedElgizery]

My understanding is that i.landsat.download is supposed to be a wapper module around i.eodag.

I therefore suggest to significantly simplify the code by just calling Module("i.eodag", start=...) with the appropriate options and flags. I do not see the need for e.g. downloader classes... You could probably create a dict that translates unified landsat options or old i.landsat.download options to the suitable options for the different providers...

Thanks for the review @ninsbl. The main problem is that each provider will have a different way to query their products... That is why we need to separate them in functions or classes. I did separate them in functions at first, but I figured it wouldn't be convenient to add more providers later on. The classes should, supposedly, provide a more clear path of what needs to be implemented for the code to work with the new provider, yet I agree there might be some code duplication, and maybe it is not worth it. Anyway, there is also an implementation that can have everything handled in one function in a sort of a clean manner. Shall we give that a try, or maybe at least have them in a separate functions?