Updates to correctly check for vertical datums, improve mosaicking, options to select surveys to process #9
Conversation
…ecific 3DEP survey only
|
|
||
|
|
||
|
|
||
| return readers, pointcloud_input_crs, extents, original_extents |
There was a problem hiding this comment.
CI is failing (https://github.com/uw-cryo/lidar_tools/actions/runs/15000341829/job/42145345525?pr=9) because this function now returns more things. Would be good to update the docstring for this function to clarify it now also returns extents and original extents. Feel free to change the test here to check for these new outputs:
Line 14 in 6b5d1fb
src/lidar_tools/dsm_functions.py
Outdated
|
|
||
| Parameters | ||
| ---------- | ||
| input_aoi : shapely.geometry.Polygon |
There was a problem hiding this comment.
Not critical, but just noting you can move these type hints into the function definition, which has the advantage of given more feedback in code editors:
def return_reader_inclusive(input_aoi: shapely.geometry.Polygon,
src_crs: pyproj.CRS,
pointcloud_resolution: int=1):
There was a problem hiding this comment.
All functions have been now updated to include typehints in the function definition via: 1f9ca8c
src/lidar_tools/dsm_functions.py
Outdated
| """ | ||
| ds = gdal.Open(raster_fn, 1) | ||
| gdal.SetConfigOption('COMPRESS_OVERVIEW', 'DEFLATE') | ||
| ds.BuildOverviews("GAUSS",[2,4,8,16,32,64,128,256,512,1024,2048,4096], |
There was a problem hiding this comment.
That's a lot of overviews! Is it necessary to go beyond 64?
There was a problem hiding this comment.
Okies, updated here: 4416695
| srcSRS=src_srs, xRes=res, yRes=res, | 8000||
| dstSRS=dst_srs, errorThreshold=tolerance, | ||
| targetAlignedPixels=True, | ||
| creationOptions=['COMPRESS=LZW','TILED=YES','COPY_SRC_OVERVIEWS=YES'], |
There was a problem hiding this comment.
| creationOptions=['COMPRESS=LZW','TILED=YES','COPY_SRC_OVERVIEWS=YES'], | |
| format='COG' |
I haven't used GDAL python bindings very much, but I wonder if that wouldn't be simpler? Would also automatically add overviews
There was a problem hiding this comment.
Yes, COG is better option, as long as we can specify gauss resampling option to create overviews
There was a problem hiding this comment.
Good point Scott, COG would be better.
As per David's note, I checked the available overview resampling methods for the COG driver (https://gdal.org/en/latest/drivers/raster/cog.html), and Gaussian resampling is not available. I am leaving this as is, and added a note to revisit this in the future!
There was a problem hiding this comment.
Looks great @ShashankBice ! Thanks for all the hard work on this. Excited to start running some more tests and integration with coincident searches. I added a couple comments, but feel free to merge whenever and we can iterate on full up changes.
|
Thank you Scott and David for the feedback and ideas! The tests still fail, need to learn that from you, Scott 😊 |
All good, I'd suggest changing to this when you get a chance. Eventually we should add tests for all these functions for two main reasons: 1. ensure outputs are correct, and 2. ensure future changes don't change existing workflows. Generally it's hard to guard against everything, but it's surprising how many issues even a few simple checks like the one below help catch issues in advance! assert len(readers) == 4
assert {'type','filename','resolution', 'polygon'} == set(readers[0].keys())
assert isinstance(crslist[0], pyproj.CRS)
assert buff_reader_extent_list[0] == (-11863800.0, 4687040.0, -11856370.0, 4691980.0)
assert original_dem_tile_grid_extent_list[0] == [-11863790.0, 4687050.0, -11856380.0, 4691970.0] |
This PR implements the following updates: