Conversation
soxofaan
left a comment
There was a problem hiding this comment.
I haven't got through this giant PR yet, but here are some comments already.
Also note that some comments probably apply to multiple other places, I didn't cover yet
|
I just stumbled on this old PR where I'm expected to give a follow-up review, but in the meantime it got marked as "has conflicts that must be resolved" (but github doesn't show me more info) Is this still a valid PR to chase? |
|
Yes, this PR is still valid. I've resolved the conflicts. @soxofaan |
|
Updated the PR again to the latest draft version. This PR is important for #468 and, if we want to have it, we should certainly have it with rc.2 and not just for the stable release to ensure the changes make sense and work as expected. |
| - `aggregate_temporal_period`: Removed unused exception `DistinctDimensionLabelsRequired`. | ||
| - `aggregate_temporal_period`: Clarified that the definition of weeks follows ISO 8601. | ||
| - `apply_polygon`: Replaced outdated usage of `raster-cube` subtype with `datacube` and dimensions. [#524](https://github.com/Open-EO/openeo-processes/issues/524) | ||
| - `array_interpolate_linear`: Apply interpolation to NaN and no-data values. |
There was a problem hiding this comment.
out curiosity: is there a reason for not only adding changelog items, but also changing the order of non-related items?
There was a problem hiding this comment.
I try to keep larger changes at the top and then the processes in alphabetical order so that it's a bit easier to follow. Sometimes changes also come in when resolving merge conflicts.
|
I did another superficial pass and I think this is fine to merge, at least to unblock other progress |
Fixes #480 and related issues:
Guide for no-data values and character encoding have been added to https://github.com/Open-EO/openeo-processes/blob/nodata/meta/implementation.md