Open-EO / openeo-python-client

Python client API for OpenEO
https://open-eo.github.io/openeo-python-client/
Apache License 2.0
143 stars 36 forks source link

Issue464 cube metadata #541

Closed VictorVerhaert closed 4 months ago

VictorVerhaert commented 4 months ago

from issue #464

VictorVerhaert commented 4 months ago

https://github.com/Open-EO/openeo-python-client/blob/498c4c7676f46d4ed6e858178cc08b1189a7699f/openeo/metadata.py#L231-L242 Currently functionality does not change due to this function. As this function checks the type and returns an object of the same class, functions like reduce_dimension (who use this function to update a metadata object) will still return a CollectionMetadata object if that was the original object.

Changing this to return a CubeMetadata changes functionality and fails some existing tests.

An alternative is to provide an optional argument to set the expected returned class, however this feels more like a quick fix and might be confusing in the future.

soxofaan commented 4 months ago

Currently functionality does not change due to this function.

I think it's fine to keep the original type of the metadata, that will indeed avoid breakage at other places.

Priority at the moment is having a base metadata class that is not implicitly tied to collection metadata (e.g. for https://github.com/Open-EO/openeo-python-client/issues/425 https://github.com/Open-EO/openeo-python-client/issues/516 https://github.com/Open-EO/openeo-python-client/issues/527 etc)

VictorVerhaert commented 4 months ago

_orig_metadata has been removed from CubeMetadata. This causes CollectionMetadata to override the _clone_and_update method in order to preserve the orig_metadata (CollectionMetadata cannot be initiated without metadata, as it should be imo).

Unless we change _clone_and_update to always return a CubeMetadata this is the only clean solution I think.

CubeMetadata tests still need to be written.

VictorVerhaert commented 4 months ago

TODO:

VictorVerhaert commented 4 months ago

Because I changed the names of the metadata tests using CollectionMetadata to test_collectionmetadata, pre-commit refactored the whole tests causing a large diff. I'll be more careful with this in the future.

I added an entry in the changelog and processed the last review.