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

Add notebook for LAS to COPC Conversion #109

Merged
merged 8 commits into from
Apr 2, 2024

Conversation

omshinde
Copy link
Collaborator

Ref #107
Tutorial for converting LiDAR LAS files to COPC format using PDAL -

Dataset used - G-LiHT Lidar Point Cloud V001

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

copc/lidar-las-to-copc.ipynb Show resolved Hide resolved
@@ -0,0 +1,660 @@
{
Copy link
Contributor

@wildintellect wildintellect Mar 1, 2024

Choose a reason for hiding this comment

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

I'm not sure the print statement is the best way to explore the PDAL object. Can we do a more python oriented method showing maybe the top level or top 2 levels of keys.

Also explain if someone wants to work with the data what would they do next, read the array values?


Reply via ReviewNB

@wildintellect
Copy link
Contributor

wildintellect commented Mar 1, 2024

A few minor comments on the notebook itself. Bigger picture I think we need to hit all the core topics. Looking at https://guide.cloudnativegeo.org/cloud-optimized-geotiffs/cogs-examples.html creation (command line) and metadata but not actual cloud-optimized usage. Comparing that to https://guide.cloudnativegeo.org/zarr/zarr-in-practice.html which creates, explores, and demos access. This might be a follow up ticket related to #91 but I would consider adding simple usage to this example.

@omshinde
Copy link
Collaborator Author

omshinde commented Mar 1, 2024

Thanks @wildintellect

A few minor comments on the notebook itself. Bigger picture I think we need to hit all the core topics. Looking at https://guide.cloudnativegeo.org/cloud-optimized-geotiffs/cogs-examples.html creation (command line) and metadata but not actual cloud-optimized usage. Comparing that to https://guide.cloudnativegeo.org/zarr/zarr-in-practice.html which creates, explores, and demos access. This might be a follow up ticket related to #91 but I would consider adding simple usage to this example.

Those are great suggestions. I can add another intro notebook focusing only on accessing a LAS file and displaying metadata using PDAL (no COPC details). Then this notebook would serve the COPC creation, access and getting point cloud stats.

@omshinde omshinde requested a review from wildintellect March 22, 2024 16:20
@omshinde
Copy link
Collaborator Author

@wildintellect can you please add @jsignell as a reviewer for this PR. Thank you.

@wildintellect wildintellect requested a review from jsignell March 25, 2024 19:54
@@ -0,0 +1,707 @@
{
Copy link
Collaborator

@jsignell jsignell Mar 25, 2024

Choose a reason for hiding this comment

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

the "from this folder" link points to a specific version of the folder. Probably better to point to use a branch rather than a sha in the url.


Reply via ReviewNB

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you. That's coming from the existing notebooks in the guide which were used as an initial template. Maybe it would be better to create separate issue for this to update all the notebooks.

copc/lidar-las-to-copc.ipynb Show resolved Hide resolved
copc/lidar-las-to-copc.ipynb Show resolved Hide resolved
copc/lidar-las-to-copc.ipynb Show resolved Hide resolved
@@ -0,0 +1,707 @@
{
Copy link
Collaborator

@jsignell jsignell Mar 25, 2024

Choose a reason for hiding this comment

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

It sounds like current versions of PDAL have just the pdal application and use the word "operations" to describe the different CLI commands.


Reply via ReviewNB

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Rephrased to make it clear. Thanks.

copc/lidar-las-to-copc.ipynb Show resolved Hide resolved
copc/lidar-las-to-copc.ipynb Show resolved Hide resolved
copc/lidar-las-to-copc.ipynb Show resolved Hide resolved
copc/lidar-las-to-copc.ipynb Show resolved Hide resolved
@omshinde omshinde requested a review from jsignell March 27, 2024 19:05
@omshinde
Copy link
Collaborator Author

Thanks @jsignell for the comprehensive review. I've updated the notebook addressing your comments. Requesting re-review.

Copy link
Collaborator

@jsignell jsignell 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 good! Nice work!

@wildintellect
Copy link
Contributor

@omshinde you're missing the navigation in

contents:
- copc/index.qmd

@omshinde
Copy link
Collaborator Author

Thanks @wildintellect ..updated the navigation link

@omshinde omshinde requested a review from jjfrench March 29, 2024 15:31
copc/lidar-las-to-copc.ipynb Show resolved Hide resolved
copc/lidar-las-to-copc.ipynb Show resolved Hide resolved
copc/lidar-las-to-copc.ipynb Show resolved Hide resolved
copc/lidar-las-to-copc.ipynb Show resolved Hide resolved
Copy link
Collaborator

@jjfrench jjfrench left a comment

Choose a reason for hiding this comment

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

Looks good to me now, @wildintellect any other comments?

@wildintellect wildintellect merged commit 5dde17d into cloudnativegeo:staging Apr 2, 2024
2 checks passed
wildintellect added a commit that referenced this pull request Apr 8, 2024
* Clarify text about cloud opt allowing partial reads (#102)

* Clean up imports

* Add back newline

* Adding writing COGs with Python notebook (#106)

* Adding writing COGs with Python notebook

* Addressing review comments

* Add overview generation and set NoData values

* Add description for overviews and predictors

* update nodata value to -32768.0

* add in-text citation

* update navigation link

* Adding link to Project  Pythia Kerchunk Cookbook (#108)

* Clarify text about cloud opt allowing partial reads (#102) (#103)

* adding ref to kerchunk cookbook

* Add notebook for LAS to COPC Conversion (#109)

* Add notebook for LAS to COPC Conversion

* Adding cli based access info

* Addressing review comments

* update navigation link for COPC notebook

* Minor updates for consistency in COPC full form

* Update environment file

* update pdal reader for copc

* update copc:true check for validation

---------

Co-authored-by: j08lue <[email protected]>
Co-authored-by: Kyle Barron <[email protected]>
Co-authored-by: Rajat Shinde <[email protected]>
Co-authored-by: rsignell <[email protected]>
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