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

Fix spacenet tutorial, allow for single bbox argument to SpatialExtent, and other fixes. #201

Merged
merged 11 commits into from
Oct 22, 2020

Conversation

lossyrob
Copy link
Member

@lossyrob lossyrob commented Oct 16, 2020

This PR adds a few fixes and enhancements based on solving the issues raised in #197 and #198.

  • Fixes the spacenet tutorial to pass in a list of bboxes to the spatial extent, rather than a single bbox.
  • Changes that tutorial to validate each of the catalogs it produces.
  • Fix an issue raised by the validation of the label items, where the label_classes property is required. This requirement is dropped in a yet-unreleased version of the schemas, so a note was added to remove after 1.0.0-beta.2's successor is released.
  • Fixed an issue with Link.get_href(), which was throwing a bad exception if a relative link had None for it's target href.
  • Allowed users to pass in a single bbox to SpatialExtent and interval to TemporalExtent to guard against the common mistake mentioned in Type check SpatialExtent bboxes parameter #198
  • Fixed an issue that caused geometry GeoJSON produced by shapely to fail PySTAC's validation. The reason for this was because the validation happened on the unmodified dict produced by a to_dict call. This can produce geometries that have their coordinates encoded with tuples instead of lists. The GeoJSON schema requires these to be lists, so the validation failed with a very cryptic message (a byproduct of using oneOf in JSON schemas - if the object doesn't match oneOf the schemas listed for a real validation failure, it instead complains about how it doesn't validate correctly against the first entry). The workaround here is to transform the dicts being passed in for validation to and from JSON serialization - if you JSON serialize a property whose value is a tuple, that will be transformed into a list for the JSON-ification. This solves the issue with shapely's mapping output, but also will solve for any case where a JSON serialization of an object would actually pass JSON schema validation, but the direct dict encoding may not.
  • Fixes the failing readthedocs by upgrading nbsphinx. This upgrade produces a lot of warnings about rendering notebook output, which was fixed by including ipython in the dev dependencies.

Fixes #197
Fixes #198

Also adds validation to the catalogs at each point so that we can
avoid this type of error moving forward.

Modifies the construction of the label items to avoid validation errors.

Fixes #197
Shapely can produce GeoJSON mappings that use tuples in the coordinate
sequences of geometries instead of lists. A direct validation of the
dict for GeoJSON will produce errors, since coordinates are expected
to be list-based. This commit adds a translation to and from JSON
serialization in order to work around this and other issue which may
appear if a JSON serialization would pass validation but the STAC dict
may not.
@lossyrob lossyrob marked this pull request as ready for review October 16, 2020 17:40
@lossyrob lossyrob requested a review from schwehr October 16, 2020 18:08
Copy link
Collaborator

@schwehr schwehr left a comment

Choose a reason for hiding this comment

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

I'm not sure best to review for pystac. I tried to err side of too many comments, questions, and suggestions.

README.md Show resolved Hide resolved
pystac/collection.py Outdated Show resolved Hide resolved
pystac/collection.py Outdated Show resolved Hide resolved
pystac/collection.py Outdated Show resolved Hide resolved
requirements-dev.txt Show resolved Hide resolved
tests/test_link.py Show resolved Hide resolved
tests/test_link.py Outdated Show resolved Hide resolved
tests/validation/test_validate.py Outdated Show resolved Hide resolved
tests/validation/test_validate.py Outdated Show resolved Hide resolved
tests/validation/test_validate.py Outdated Show resolved Hide resolved
- Use consistent datetime
- Don't use license if not required
- Make descriptions different than IDs
- Specify why we're setting th href
- use consistent datetime
- better docstring
- use different ID and description
- Get item link explicitly
- Use less decimal places
- use simpler parameters.
- comment why the trailing , is needed.
@lossyrob lossyrob merged commit 1fcfb44 into develop Oct 22, 2020
@lossyrob lossyrob deleted the rde/fix-doc-build branch October 22, 2020 20:57
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.

Type check SpatialExtent bboxes parameter Tutorial code constructing SpatialExtents is incorrect
2 participants