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

Reduce_temporal throws MetadataException after load_stac #567

Closed
VictorVerhaert opened this issue Jun 4, 2024 · 7 comments
Closed

Reduce_temporal throws MetadataException after load_stac #567

VictorVerhaert opened this issue Jun 4, 2024 · 7 comments
Labels

Comments

@VictorVerhaert
Copy link
Contributor

When using load_stac, the python client tries to read metadata from the provided stac. The only metadata currently being read is bands, so no spatial or temporal metadata.

When the python client fails to read metadata from the stac, metadata is simply set to None.
These lines check is metadata is present and have default behaviour otherwise:

def reduce_temporal(self, reducer: Union[str, PGNode, typing.Callable, UDF]) -> DataCube:
"""
Shortcut for :py:meth:`reduce_dimension` along the temporal dimension
:param reducer: "child callback" function, see :ref:`callbackfunctions`
"""
return self.reduce_dimension(
dimension=self.metadata.temporal_dimension.name if self.metadata else "t",
reducer=reducer,
)

and
metadata=self.metadata.reduce_dimension(dimension_name=dimension) if self.metadata else None,

The problem arises however when metadata IS being read from the STAC. As only bands are read cube.metadata is not None, but it does not contain a temporal dimension resulting in a MetadataException originating for example from:

@property
def temporal_dimension(self) -> TemporalDimension:
if not self.has_temporal_dimension():
raise MetadataException("No temporal dimension")
return self._temporal_dimension

The solution is threefold I think:

  1. Not only check if the metadata is not None but also whether it contains a temporal dimension
  2. Implement reading a temporal dimension (and spatial) from stac metadata
  3. Catch metadata related errors and throw them as warnings (Discussion) as this blocks the execution of a batch job while it would run fine.
@soxofaan
Copy link
Member

soxofaan commented Jun 4, 2024

as discussed, a workaround would be to keep the openeo package below 0.29 for now

@VictorVerhaert
Copy link
Contributor Author

Another workaround is to add the following:
cube.metadata = cube.metadata.add_dimension("t", label=None, type="temporal")

@soxofaan
Copy link
Member

soxofaan commented Jun 5, 2024

Started with a PR at #568

@soxofaan
Copy link
Member

I already merged an initial fix that avoids throwing the MetadataException, so at least that can already be included in the next release

next steps are properly detecting real properties of the temporal dimension

@clausmichele
Copy link
Member

clausmichele commented Jul 17, 2024

@soxofaan @VictorVerhaert this method parses already correctly the dimensions, why not re using that? It seems we are trying to re-implement something that already exists:

def _parse_dimensions(cls, spec: dict, complain: Callable[[str], None] = warnings.warn) -> List[Dimension]:

@clausmichele
Copy link
Member

@soxofaan I created a PR here: #591 . Currently it solves the issue I have, but there are probably other scenarios that could be covered (currently it makes only a difference for STAC Collections having cube:dimensions available.

soxofaan added a commit to clausmichele/openeo-python-client that referenced this issue Aug 20, 2024
…imension name in `load_stac`

-  move logic to _StacMetadataParser (less logic nesting)
- improve test coverage (and add DummyStacDictBuilder utility)
soxofaan added a commit to clausmichele/openeo-python-client that referenced this issue Aug 20, 2024
soxofaan added a commit to clausmichele/openeo-python-client that referenced this issue Aug 20, 2024
…ual temporal dimension name in `load_stac`
soxofaan added a commit to clausmichele/openeo-python-client that referenced this issue Aug 20, 2024
… of actual temporal dimension name in `load_stac`
soxofaan added a commit to clausmichele/openeo-python-client that referenced this issue Aug 21, 2024
…tection of actual temporal dimension name in `load_stac`
soxofaan added a commit that referenced this issue Aug 21, 2024
…in `load_stac`

-  move logic to _StacMetadataParser (less logic nesting)
- improve test coverage (and add DummyStacDictBuilder utility)
@soxofaan
Copy link
Member

I think this can now be closed with #591 being merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants