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.

Sign up for GitHub

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

Jump to bottom

54 supporting other lidar interpolation options #69

Merged
merged 4 commits into from
Apr 27, 2022
Merged

54 supporting other lidar interpolation options #69

merged 4 commits into from
Apr 27, 2022

Conversation

rosepearson
Copy link
Owner

@rosepearson rosepearson commented Apr 22, 2022 •
edited
Loading

Add support for calculating the mean, median or IDW of LiDAR points during the DEM generation process:

  • Make code change
  • Update tests
    • Update / create new tests
    • Ensure these tests have the expected behaviour
    • Test locally and ensure tests are passing
  • Ensure tests passing on GH Actions
  • Update documentation
    • Doc strings
    • Wiki
  • Update package version (if needed)

@rosepearson rosepearson linked an issue Apr 22, 2022 that may be closed by this pull request
Adding mean and median interpolation options in DEM generation process #54
Closed
@rosepearson rosepearson requested a review from Martin20494 April 22, 2022 04:24
@rosepearson
Copy link
Owner Author

rosepearson commented Apr 22, 2022 •
edited
Loading

Hi @Martin20494,
I've implemented support for calculating the mean, median or IDW from LiDAR points during the DEM generation process. I've done this by adding a new instruction file parameter 'lidar_interpolation_method with accepted values of 'idw', 'mean' and 'median'.

Do you mind reviewing my PR and accepting these changes if you are happy with them? I'll create a meeting for next Tuesday to run through what's involved in performing a PR.

Also should we add support for the min and max as well?

@Martin20494
Copy link
Collaborator

Hi @rosepearson,
That's awesome! Thank you so much Rose for doing this. I might need more than a week to have a try with it as I am in between of writing some reports, but will try my best! I think we also need min and max, but I will discuss with Matt about this.

@rosepearson
Copy link
Owner Author

rosepearson commented Apr 25, 2022 •
edited
Loading

Awesome @Martin20494 RE Min and Max. I was wondering. I'll add them now. Let me know if there is any other averaging you may want too and I can add them prior to our meeting tomorrow.

I've had a look at the options provided by OpenTopography and will add all of them. This will mean adding min, max, mean, median, std, and point count.

image

Rose

@Martin20494
Copy link
Collaborator

Awesome! Thank you Rose @rosepearson

@rosepearson rosepearson changed the base branch from main to 54-adding-mean-and-median-interpolation-options-in-dem-generation-process April 27, 2022 02:05
rosepearson
rosepearson commented Apr 27, 2022
View reviewed changes
Copy link
Owner Author

@rosepearson rosepearson left a comment

Choose a reason for hiding this comment

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

I have spoken to @Martin20494 and he is happy with this as a first go at adding support for basic interpolation methods. He mentioned we may want to support another method from a paper in the future.

src/geofabrics/dem.py Show resolved Hide resolved
@rosepearson rosepearson merged commit d97f005 into 54-adding-mean-and-median-interpolation-options-in-dem-generation-process Apr 27, 2022
@rosepearson rosepearson deleted the 54-supporting-other-lidar-interpolation-options branch April 27, 2022 04:52
rosepearson added a commit that referenced this pull request Apr 27, 2022
@rosepearson
63 Adding metadata and data source classification to produced netCDF …
9fbb041 
…files (#68)

* Added the netcdf4 library to the environment - needed for writing out grouped netcdf files.

* Change to using XArray Datasets

* Use a separate version.py, update the setup.py - allows version to be referenced when writing out DEMs

* Added a new variable to the DEM - source classification - and updated how these are read in to ensure rasterio doesn't add a 'band' coordinate. Also added some explicit garbage collection as the freeing of memory associated with netCDF files is not being done otherwise.

* Updated tests with new benchmarks with datasets not dataarrays

* Merged in issue 54 - supporting other lidar interpolation options (#69)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Martin20494 Martin20494 Awaiting requested review from Martin20494

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Adding mean and median interpolation options in DEM generation process
2 participants
@rosepearson @Martin20494