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 for Pointcloud extensions #176

Merged
merged 13 commits into from
Sep 4, 2020

Conversation

chelm
Copy link
Contributor

@chelm chelm commented Aug 21, 2020

This PR adds support pointclouds as an item extension (https://github.com/radiantearth/stac-spec/tree/master/extensions/pointcloud).

Example usage:

item.ext.enable(pystac.Extensions.POINTCLOUD)
item.ext.pointcloud.apply(1000000, 'lidar', 'laszip', ept['schema'])

This work was done as part of STAC Sprint #6 and all tests are passing. Open to any suggestions or changes on this as well.

@chelm
Copy link
Contributor Author

chelm commented Aug 21, 2020

oh no my builds failed!?

@chelm
Copy link
Contributor Author

chelm commented Aug 21, 2020

oh flake issues, will fix

@chelm
Copy link
Contributor Author

chelm commented Aug 21, 2020

There we go!

@whatnick
Copy link
Contributor

oh flake issues, will fix

I have been running ./scripts/test , got caught by flake8 a couple of times.

Copy link
Member

@lossyrob lossyrob left a comment

Choose a reason for hiding this comment

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

Thanks! This is looking good. There's a couple of small things mentioned inline.

There's a comment on considering whether or not statistics and schemas should be represented by their own objects. This happens in other extensions, and I think has a benefit of consistency and usability when constructing objects for properties. However I can also imagine some downsides, so happy to talk this through or lean on your experience as a user of the extension to decide which way to go with it.

One other thing - could you add a CHANGELOG entry? This feature will be released in 0.5.3 which currently doesn't have a section, so you could create one for this 'Added' feature.

pystac/extensions/pointcloud.py Show resolved Hide resolved
pystac/extensions/pointcloud.py Outdated Show resolved Hide resolved
pystac/extensions/pointcloud.py Show resolved Hide resolved
pystac/extensions/pointcloud.py Outdated Show resolved Hide resolved
tests/extensions/test_pointcloud.py Show resolved Hide resolved
@chelm
Copy link
Contributor Author

chelm commented Aug 25, 2020

Thanks @lossyrob for the comments. I'll get on them all this week as a i have time

@chelm
Copy link
Contributor Author

chelm commented Sep 4, 2020

Ok i've resolved the comments and this extension is feeling pretty good. Sorry it took me so long to get to this.

Now i just need to look at why the PR checks failed.

@lossyrob
Copy link
Member

lossyrob commented Sep 4, 2020

Looks great, thanks!

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.

3 participants