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

fix: update UDPs description and replace broken image links with raw github links #49

Closed
wants to merge 21 commits into from

Conversation

rahmandawibowo-vito
Copy link
Contributor

@rahmandawibowo-vito rahmandawibowo-vito commented Oct 29, 2024

I notice there are some issues with the OpenEO UDP description:

@rahmandawibowo-vito rahmandawibowo-vito changed the title fix: update UDPs description and replace broken image links with raw github links Draft: fix: update UDPs description and replace broken image links with raw github links Oct 29, 2024
@rahmandawibowo-vito rahmandawibowo-vito changed the title Draft: fix: update UDPs description and replace broken image links with raw github links fix: update UDPs description and replace broken image links with raw github links Oct 29, 2024
@rahmandawibowo-vito
Copy link
Contributor Author

Specifically for the world cereal UDP, the description is not completed yet plus there is no accompanied markdown file either. Is there any references I can use to update the description @HansVRP?

algorithm_catalog/bap_composite.json Outdated Show resolved Hide resolved
![cloud-shadow-artifacts-bap.png](https://raw.githubusercontent.com/ESA-APEx/apex_algorithms/main/algorithm_catalog/bap_composite_files/cloud-shadow-artifacts-bap.png)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this is a structural solution to this problem: you can't expect users to produce this URL because it doesn't exist at the time they write the description, so they can not verify their work.

Can't this problem be solved when rendering the description? All the necessary bits should be available to produce the correct URL at render time I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually at the moment we have added a logic to build this image url automatically, but the output is not always working. This is because there is a case where the image directory name doesn't have the same structure with the rest of the UDP here https://github.com/ESA-APEx/apex_algorithms/tree/main/algorithm_catalog/sentinel1_stats (in this PR, I have renamed the folder). If we can make this consistent, surely we don't need to rewrite the image url directly on the description.

![s1_stats](https://raw.githubusercontent.com/ESA-APEx/apex_algorithms/main/algorithm_catalog/sentinel1_stats/sentinel1.png)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
![s1_stats](https://raw.githubusercontent.com/ESA-APEx/apex_algorithms/main/algorithm_catalog/sentinel1_stats/sentinel1.png)
![s1_stats](https://raw.githubusercontent.com/ESA-APEx/apex_algorithms/main/algorithm_catalog/sentinel1_stats_files/sentinel1.png)

If I understand correctly then

@soxofaan
Copy link
Contributor

soxofaan commented Nov 5, 2024

Minor update on this PR:
I'm afraid it's hard to approve/merge this PR as a whole, some fixes can be applied without further discussion, but other need some more thinking or discussion.
I'll see if I can cherry pick some things already.

cc @HansVRP @VincentVerelst

@soxofaan
Copy link
Contributor

soxofaan commented Nov 5, 2024

I just cherry-picked the issue with feature branch references in 523a16f

@soxofaan
Copy link
Contributor

soxofaan commented Nov 5, 2024

Regarding the remaining changes from this PR, I only think these changes of the UDP descriptions still make sense to look at (e.g. as new PR):

@soxofaan soxofaan marked this pull request as draft November 5, 2024 11:06
@soxofaan
Copy link
Contributor

soxofaan commented Nov 5, 2024

to better reflect the state of this PR, I changed it to a draft PR

@rahmandawibowo-vito
Copy link
Contributor Author

Regarding the remaining changes from this PR, I only think these changes of the UDP descriptions still make sense to look at (e.g. as new PR):

Yes this PR seems larger that expected. I'll close this PR and create new PRs to fix the UDP individually.

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