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

Sentinel 1 Stats UDP ported to APEX #36

Merged
merged 11 commits into from
Oct 18, 2024
Merged

Sentinel 1 Stats UDP ported to APEX #36

merged 11 commits into from
Oct 18, 2024

Conversation

Pratichhya
Copy link
Contributor

@Pratichhya Pratichhya commented Oct 3, 2024

A User Defined Process (UDP) has been created for the Sentinel 1 Stats workflow. Ref: https://github.com/Open-EO/openeo-community-examples/blob/main/python/Sentinel1_Stats/Sentinel1_Stats.ipynb

Included is a generate.py file that generates a process graph and saves it as a .json file. However, this process is not yet published or accessible as a shareable service. Within this file, I adopted the same method for establishing connection as for others

In the algorithm catalog JSON, a link to an OpenEO test result has been provided as a shareable link, for example.

Please note that this link may need to be updated.

@Pratichhya Pratichhya linked an issue Oct 3, 2024 that may be closed by this pull request
@Pratichhya Pratichhya requested review from soxofaan and jdries October 3, 2024 10:14
@Pratichhya
Copy link
Contributor Author

also could you please help me understand the cause of final action failure?

algorithm_catalog/sentinel1_stats.json Outdated Show resolved Hide resolved
benchmark_scenarios/sentinel1_stats.json Outdated Show resolved Hide resolved
openeo_udp/sentinel1_stats/generate.py Show resolved Hide resolved
openeo_udp/sentinel1_stats/generate.py Show resolved Hide resolved
@HansVRP
Copy link
Contributor

HansVRP commented Oct 14, 2024

@jdries The only failing unit tests seems to be related to the process graph not being on the actual repo, which should be resolved after the merge?

@HansVRP HansVRP requested a review from jdries October 14, 2024 09:04
@soxofaan
Copy link
Contributor

soxofaan commented Oct 14, 2024

also could you please help me understand the cause of final action failure?

from https://github.com/ESA-APEx/apex_algorithms/actions/runs/11323766302/job/31487115522?pr=36

FAILED tests/test_scenarios.py::test_lint_scenario[sentinel1_stats] - requests.exceptions.HTTPError: 404 Client Error: Not Found for url: https://raw.githubusercontent.com/ESA-APEX/apex_algorithms/main/openeo_udp/sentinel1_stats.json

In your benchmark definition you use URL https://raw.githubusercontent.com/ESA-APEX/apex_algorithms/main/openeo_udp/sentinel1_stats.json which does not exist yet as this PR is not merged yet.

This raises a bit a problem with this PR based approach at submitting algorithms: in the benchmark definition you have to point to the URL of the UDP, but what should you use? The URL of the feature branch being used to create the PR, or the URL of where the UDP will finally live after merging the PR (main branch)?

Two quick solutions:

  • First make a PR of the UDP and algorithm definition and get that merged. Do the benchmark with a different PR
  • Do a first PR with UDP, algorithm and benchmark definition, where you point to the UDP with the feature branch based URL. When that is merged, optionally: follow-up PR to update URLs

@soxofaan
Copy link
Contributor

@HansVRP HansVRP requested a review from soxofaan October 14, 2024 18:19
@HansVRP
Copy link
Contributor

HansVRP commented Oct 16, 2024

@jdries @soxofaan I guess its ready for merge

Copy link
Contributor

@soxofaan soxofaan left a comment

Choose a reason for hiding this comment

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

yes I think it's fine to merge now

@jdries jdries merged commit a0114be into main Oct 18, 2024
1 check passed
@jdries
Copy link
Contributor

jdries commented Oct 18, 2024

Ok, pushed the button, great to have another algorithm!

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.

Sentinel-1 stats
4 participants