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

Support extensions defining top-level properties on collections #191

Merged
merged 6 commits into from
Feb 6, 2024

Conversation

nkleinbaer
Copy link
Contributor

@nkleinbaer nkleinbaer commented Feb 5, 2024

Description:
Currently, extensions that define additional top-level properties on collection objects (e.g. item-assets) are effectively unusable because the CollectionSerializer.db_to_stac only allows the 'core' properties to be passed through. This PR changes that to allow arbitrary additional props.

That method also includes code to populate the 'core' properties with default values if they are not present. At one point while working on this I had deleted that code and noticed that doing so did not break any tests. So I added test_collection_defaults to ensure that any changes to that behavior will be caught going forward.

PR Checklist:

  • Code is formatted and linted (run pre-commit run --all-files)
  • Tests pass (run make test)
    - [ ] Documentation has been updated to reflect changes, if applicable
  • Changes are added to the changelog

@nkleinbaer nkleinbaer marked this pull request as ready for review February 5, 2024 17:27
Copy link
Contributor Author

@nkleinbaer nkleinbaer left a comment

Choose a reason for hiding this comment

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

Initially I thought it would be necessary to use stac_pydantic.Collection, which disallows empty strings for certain props ... that proved not to be necessary, but I left these changes hanging around. 3c7b581 reverts back to the original defaults.

@jonhealy1 jonhealy1 self-requested a review February 6, 2024 02:43
Copy link
Collaborator

@jonhealy1 jonhealy1 left a comment

Choose a reason for hiding this comment

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

This looks really good. Thank you.

@jonhealy1 jonhealy1 merged commit 610d800 into stac-utils:main Feb 6, 2024
4 checks passed
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