Skip to content

Conversation

@ElienVandermaesenVITO
Copy link
Contributor

With the changes of Open-EO/openeo-geotrellis-extensions#406, the metadata of the assets have a bands will a field bands that contains statistics instead of the raster:bands field. When trying to download the results (j-25102008133547d8a8deeb00481658b0), an error happens: TypeError("Band.__new__() got an unexpected keyword argument 'statistics'") (r-2510211238334676a70a5ae8dd8ae81a)
Adding statistics to the Band class should fix this

@EmileSonneveld
Copy link
Member

@codex review

@chatgpt-codex-connector
Copy link

Codex Review: Didn't find any major issues. Chef's kiss.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@soxofaan
Copy link
Member

As mentioned IRL, I think this fix is at the wrong level. The Bands class in the python client is intentionally a very simple data class, to just cover the bare openEO-oriented band metadata that are relevant for the client code (which is currently just name, and maybe common name). Statistics is a bit too advanced at the moment and STAC-extension-specific.
If there is reason to increase the scope of this class, I would immediately go for a pystac based one, instead having to deal with incremental ad-hoc additions each time some lack of extra metadata support is uncovered.

If I understand correctly, the source of the error is this construct in openeo-geopyspark-driver:

                    asset["bands"] = [Band(**b) for b in asset["bands"]]

A better fix would be to avoid the naive "just push in everything" approach of Band(**)
or to make a richer Band subclass that does support extra metadata aspects provided by openeo-geopyspark-driver

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