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

Rewrite Black Sea GeoJSON to conform to current schema #7

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

pont-us
Copy link
Member

@pont-us pont-us commented Nov 2, 2022

License URLs for the variables are currently set to UNKNOWN, but all other fields have been populated from the previous GeoJSON or from other data in the black-sea directory in this repository. The GeoJSON in this PR has been validated against the current version of the DeepESDL dataset schema.

Content is not yet complete: placeholders are used for
sources.data_url and sources.license_url in the variable properties.
@pont-us pont-us requested review from forman and AliceBalfanz and removed request for forman and AliceBalfanz November 2, 2022 15:01
Copy link
Contributor

@forman forman left a comment

Choose a reason for hiding this comment

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

I believe, we should meet to consolidate the GeoJSON structure.

One way to move forward, is to only specify a subset of properties specified by CF Convention and ones defined by https://wiki.esipfed.org/Attribute_Convention_for_Data_Discovery_1-3 so we can use them as metadata directly.

Comment on lines +82 to +83
"valid_min": -3.4028234663852886e+38,
"valid_max": 3.4028234663852886e+38,
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these the true valid_min and valid_max? Note that they replace fill_value.
I'm not sure why we include them.

If we want useful limits for the tiles in xcube Viewer, we should use color_value_min and color_value_max.

Copy link
Member Author

Choose a reason for hiding this comment

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

These are true in the sense of being the minimum and maximum representable values for the float32 data type used for this variable. But I couldn't find any documentation of the semantics of these fields, so I don't know if this was the intended sense of "valid".

Comment on lines +170 to +171
"valid_min": -3.4028234663852886e+38,
"valid_max": 3.4028234663852886e+38,
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment above.

black-sea/cube.geojson Outdated Show resolved Hide resolved
black-sea/cube.geojson Outdated Show resolved Hide resolved
black-sea/cube.geojson Outdated Show resolved Hide resolved
black-sea/cube.geojson Outdated Show resolved Hide resolved
The content of the "source" item in the per-variable metadata object
duplicates the content of the "remarks" item in the first (and only)
"sources" object. Removing "source" to avoid redundancy.
@pont-us
Copy link
Member Author

pont-us commented Nov 3, 2022

I've opened another pull request for the schema and template at deepesdl/dataset-spec#5, so that I can update them in parallel with the changes happening here.

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