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 Lidar tutorial by @mazingaro #10

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

veroandreo
Copy link
Contributor

@veroandreo veroandreo commented Aug 27, 2024

It's currently in .ipynb format, so it'll require conversion to .qmd and formatting fixes. It also requires a link to the data so it can be fully reproduced.

"\n",
"This tutorial can be run locally. You need to have GRASS GIS 8.4+ and Jupyter installed. The processes demands some specific libraries. Be sure to have numpy, pandas, geopandas, pdal, python-pdal and tiledbb with pybabylonjs. \n",
"\n",
"The first thing we need to do for any of the cases we'll see further on, is to import GRASS GIS python packages. In order to do so, we need to add GRASS GIS python package to PATH. Let's see how we do that."
Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe this can be moved below, where this is actually happening?

jupyter_lidar_inspection.ipynb Outdated Show resolved Hide resolved
jupyter_lidar_inspection.ipynb Outdated Show resolved Hide resolved
jupyter_lidar_inspection.ipynb Outdated Show resolved Hide resolved
Comment on lines +190 to +208
{
"cell_type": "code",
"execution_count": null,
"id": "f67c8b8f",
"metadata": {},
"outputs": [],
"source": [
"# Let's check the environment settings: with os.environ[] we can see all of them\n",
"os.environ['GRASS_OVERWRITE'] # This environent variable shows us if we can overwrite our outputs. "
]
},
{
"cell_type": "markdown",
"id": "71e59b8d",
"metadata": {},
"source": [
"- Question: What can wee see from the output above? Is it the same as we are used in GRASS?"
]
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure about this, maybe not needed?

Choose a reason for hiding this comment

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

The whole part about the GRASS_OVERWRITE? I think that is really useful for GRASS users to know, that now they have to be careful here, as overwriting is set by default.

jupyter_lidar_inspection.ipynb Outdated Show resolved Hide resolved
"id": "4ccb7f8b",
"metadata": {},
"source": [
"Is that composite better or worse than an interpolated, raw one for your future data analysis?"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

consider adding some explanation

Comment on lines +766 to +767
"# We can see differences on microtopography:\n",
"# Let's denoise and see how does local noise affect the elevation mode\n",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
"# We can see differences on microtopography:\n",
"# Let's denoise and see how does local noise affect the elevation mode\n",
"# We can see differences on microtopography:\n",
"# Let's denoise and see how does local noise affect the elevation model\n",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, this requires guidance/explanations

"id": "fd959956",
"metadata": {},
"source": [
"Create a temporary directory or if you created one in the beginning just use that."
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please explain why you do these steps, and maybe better just use here the same temp directory you created to avoid confusion, i.e., keep it simple

jupyter_lidar_inspection.ipynb Outdated Show resolved Hide resolved
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