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

139-slope-collection #155

Merged
merged 13 commits into from
Sep 12, 2024
Merged

139-slope-collection #155

merged 13 commits into from
Sep 12, 2024

Conversation

kvantricht
Copy link
Contributor

Load slope from global collection when on CDSE backend

Copy link
Contributor

@GriffinBabe GriffinBabe left a comment

Choose a reason for hiding this comment

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

Some minor remarks, except for the OpenEO client version

Also I would move the slope computation directly to the PatchFeatureExtractor superclass in GFMAP in the future, as it can be useful to everyone

@@ -41,7 +41,7 @@ dependencies = [
"numpy<2.0.0",
"netcdf4<=1.6.4",
"h5netcdf>=1.1.0",
"openeo>=0.29.0",
"openeo>=0.31.0",
Copy link
Contributor

@GriffinBabe GriffinBabe Sep 12, 2024

Choose a reason for hiding this comment

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

I would wait for this ticket to close before changing the OpenEO version

Open-EO/openeo-gfmap#151

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok but we can't merge this PR without bumping the version because the new slope collection cannot be loaded with 0.29.0. Something in the python client changed and the former version cannot work with it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, I'll try to close this issue GFMAP today

Copy link
Contributor

Choose a reason for hiding this comment

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

@kvantricht Nevermind sorry! this issue will come with Version 0.32 and the feature is not yet merged on the client repository

@kvantricht
Copy link
Contributor Author

Some minor remarks, except for the OpenEO client version

Also I would move the slope computation directly to the PatchFeatureExtractor superclass in GFMAP in the future, as it can be useful to everyone

yeah but the way it's implemented now (especially with the required change to force computation at 20m) is probably not generic enough. I think what would be better instead is a slope (or terrain attributes) process in openEO, which can then be accessed in GFMAP in a transparent way.

@kvantricht kvantricht merged commit 6767f93 into main Sep 12, 2024
4 checks passed
@kvantricht kvantricht deleted the 139-slope-collection branch September 12, 2024 11:52
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.

2 participants