Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

i.eodag: add query, print, and footprints options #1136

Merged
merged 47 commits into from
Jul 10, 2024

Conversation

HamedElgizery
Copy link
Contributor

@HamedElgizery HamedElgizery commented Jul 1, 2024

  • Add query option, from which one can set extra querayables for searching.

  • Add print option to allow printing of the following in JSON format:

    • Current EODAG configuration.
    • Available products.
    • Available products for a given provider.
    • Available providers.
    • Available providers offering a specific product.
    • Queryables for a given provider and/or product.

Note: Queryables from the above list can then be used in the query option.

  • Add foorprints option to allow saving the scenes in a single combined footprint vector map.

@HamedElgizery HamedElgizery changed the title i.eodag: add query and print options i.eodag: add query, print, and footprints options Jul 1, 2024
@veroandreo veroandreo requested a review from ninsbl July 2, 2024 12:50
Copy link
Contributor

@veroandreo veroandreo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some comments and questions regarding flags and footprint option. Also, several docstrings have the "EO products to be sorted" as parameter description, it doesn't seem right in all cases, it is maybe a left over of copy-pasting, please have a look.

Also some more use cases and examples would be desirable in the html

src/imagery/i.eodag/i.eodag.html Outdated Show resolved Hide resolved
src/imagery/i.eodag/i.eodag.py Outdated Show resolved Hide resolved
src/imagery/i.eodag/i.eodag.py Outdated Show resolved Hide resolved
src/imagery/i.eodag/i.eodag.py Outdated Show resolved Hide resolved
# %option
# % key: query
# % label: Extra searching parameters to use in query
# % description: Note: Make sure to use provided options when possible, otherwise the values mignt not be recognized
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the message here? I do not understand

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I am trying to convey is that if you are able to use one of the provided i.eodag options e.g. start, end, producttype, provider, etc... Then you should use them instead of passing them through the query option, because they might cause unexpected problems.

Shall I just type this, if it is more clear? or is it too wordy?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand right, this option is to use searching parameters not provided for by the other filter parameters? Perhaps turning it around:

"Query using extra searching (filter?) parameters not provided for by the provided filter options"

Would it be possible to check if the user uses query parameters that are already available as parameter option, and issue a warning if the user uses these parameters?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This dhould be made clear in the manual...

src/imagery/i.eodag/i.eodag.py Outdated Show resolved Hide resolved
src/imagery/i.eodag/i.eodag.py Outdated Show resolved Hide resolved
src/imagery/i.eodag/i.eodag.py Outdated Show resolved Hide resolved
src/imagery/i.eodag/i.eodag.py Show resolved Hide resolved
src/imagery/i.eodag/i.eodag.py Outdated Show resolved Hide resolved
HamedElgizery and others added 10 commits July 2, 2024 16:49
Co-authored-by: Veronica Andreo <veroandreo@gmail.com>
Co-authored-by: Veronica Andreo <veroandreo@gmail.com>
Co-authored-by: Veronica Andreo <veroandreo@gmail.com>
Co-authored-by: Veronica Andreo <veroandreo@gmail.com>
Co-authored-by: Veronica Andreo <veroandreo@gmail.com>
Co-authored-by: Veronica Andreo <veroandreo@gmail.com>
Copy link
Member

@ninsbl ninsbl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, I have to review on a mobile phone, which is somewhat limiting. So, i may have overlooked something. Anyway, please see my comments in the code...

src/imagery/i.eodag/i.eodag.py Outdated Show resolved Hide resolved
src/imagery/i.eodag/i.eodag.py Outdated Show resolved Hide resolved
src/imagery/i.eodag/i.eodag.py Outdated Show resolved Hide resolved
src/imagery/i.eodag/i.eodag.py Outdated Show resolved Hide resolved
src/imagery/i.eodag/i.eodag.py Outdated Show resolved Hide resolved
src/imagery/i.eodag/i.eodag.py Show resolved Hide resolved
Copy link
Contributor

@veroandreo veroandreo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just tested different use cases. In generally, it works as expected, except for the query option. I tried with OrbitDirection and start, but nothing gets filtered really with those... so, I'm not sure it actually works or how it is supposed to work (the queryables for S2 will return mostly the same values, because they are like organization, instrument and so on...). Perhaps if multiple products are passed to producttype, it makes more sense??

This is what I tried with queryables:

i.eodag -l start=2022-05-01 end=2022-06-01 clouds=50 producttype=S2_MSI_L2A provider=cop_dataspace query="start=2022-05-20"

i.eodag.py -l start=2022-05-01 end=2022-06-01 clouds=50 producttype=S2_MSI_L2A provider=cop_dataspace query="orbitDirection=DESCENDING"

Try with and without the query option in NC project with this region setting:

n=228510
s=214980
w=630000
e=645000

In general, I'm in to merge this one as it got too long already (maybe smaller PRs are easier to review and test), and if there's something else to fix, create a new PR.

@ninsbl
Copy link
Member

ninsbl commented Jul 10, 2024

I think the additional queryables need to be used in the crunch function on the returned initial surchresults, not during initial surch.

As for implementing the queriables, please consider the different operators and possible use of multiple values. Please double check also my earlier suggestions.

Copy link
Member

@ninsbl ninsbl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @veroandreo that we should merge and then improve from the current state.

I still think that however, that extraction and deletion of archives is more appropriately implemented with flags. In any case, if a settings file gets priority over options/flags should be made clear in the manual...

A next step should be to add test cases that do not require login credentials, like for printing, filtering and so on..

@HamedElgizery
Copy link
Contributor Author

I've just tested different use cases. In generally, it works as expected, except for the query option. I tried with OrbitDirection and start, but nothing gets filtered really with those... so, I'm not sure it actually works or how it is supposed to work (the queryables for S2 will return mostly the same values, because they are like organization, instrument and so on...).

Thanks for the testing! I am currently working on fixing the issue.

In general, I'm in to merge this one as it got too long already (maybe smaller PRs are easier to review and test), and if there's something else to fix, create a new PR.

Makes sense, I will be pushing the fix on the next PR if we agree on that, which will also extend on the current query functionality.

@veroandreo veroandreo merged commit 4a38040 into OSGeo:grass8 Jul 10, 2024
7 checks passed
@HamedElgizery HamedElgizery deleted the eodag_queryables branch July 11, 2024 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants