Skip to content

Conversation

@hrodmn
Copy link
Member

@hrodmn hrodmn commented Dec 24, 2024

Description

First draft! I am so sorry for the monstrous line count but that's what I get for using pytest-recording and checking in the example json files.

Most of the interesting additions are in metadata.py which has the logic for extracting the metadata from parquet files.

You can browse the example metadata here: https://radiantearth.github.io/stac-browser/#/external/raw.githubusercontent.com/stactools-packages/overture-maps/feat/stac/examples/catalog.json

Checklist

  • Tests pass (run scripts/test)
  • Documentation has been updated to reflect changes, if applicable
  • Examples have been updated to reflect changes, if applicable
  • Changes are added to the CHANGELOG, if applicable

@hrodmn hrodmn marked this pull request as ready for review December 24, 2024 13:10
Copy link

@kylebarron kylebarron left a comment

Choose a reason for hiding this comment

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

Overall this looks really good! I love the use of dataclasses and pystac

PARTITION_FORMAT,
)

COLLECTION_DESCRIPTION_FORMAT = (

Choose a reason for hiding this comment

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

Nit: I'd suggest a multi line string to make this easier to read

COLLECTION_DESCRIPTION_FORMAT = """\
## Overture Maps - {theme}

This collection contains items for the {theme} theme.

{theme_description}

etc

def theme_description(self) -> str:
"""Scrape theme description from OvertureMaps/docs repo"""
# Get the raw content
raw_url = f"https://raw.githubusercontent.com/OvertureMaps/docs/refs/heads/main/docs/guides/{self.theme.value}.mdx"

Choose a reason for hiding this comment

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

If this is mdx, there may be some elements that don't render correctly in standard markdown.

)

# version extension
VersionExtension.ext(item, add_if_missing=True)

Choose a reason for hiding this comment

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

Is this supposed to be assigned to something? Or does this have side-effects?

Choose a reason for hiding this comment

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

It has side effects, and the "more modern" way to do this is:

Suggested change
VersionExtension.ext(item, add_if_missing=True)
item.ext.add("version");

The simpler extension interface was introduced in v1.9.0, led by @jsignell.

Comment on lines +427 to +430
# set primary geometry type to first geometry_type in geoparquet metadata
item.ext.table.primary_geometry = self.geo_metadata["columns"]["geometry"][
"geometry_types"
][0]

Choose a reason for hiding this comment

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

If there's a list of multiple geometry types, you really can't just pick one. It could be a mix of polygon and multi polygon and you'll lose that information

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