-
Notifications
You must be signed in to change notification settings - Fork 20
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
Make Raster.to_pointcloud()
modular and add Raster.get_mask()
for memory-efficient operations requiring mask
#501
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rhugonnet
changed the title
Make
Make Mar 8, 2024
Raster.to_points()
modular and add Raster._load_only_mask
for memory-efficient operation requiring only valid maskRaster.to_pointcloud()
modular and add Raster._load_only_mask
for memory-efficient operation requiring only valid mask
rhugonnet
changed the title
Make
Make Mar 8, 2024
Raster.to_pointcloud()
modular and add Raster._load_only_mask
for memory-efficient operation requiring only valid maskRaster.to_pointcloud()
modular and add Raster.get_mask
for memory-efficient operations requiring only valid mask
rhugonnet
changed the title
Make
Make Mar 9, 2024
Raster.to_pointcloud()
modular and add Raster.get_mask
for memory-efficient operations requiring only valid maskRaster.to_pointcloud()
modular and add Raster.get_mask()
for memory-efficient operations requiring mask
3 tasks
adehecq
reviewed
Mar 19, 2024
adehecq
reviewed
Mar 19, 2024
adehecq
reviewed
Mar 19, 2024
adehecq
reviewed
Mar 19, 2024
adehecq
reviewed
Mar 19, 2024
adehecq
reviewed
Mar 19, 2024
adehecq
reviewed
Mar 19, 2024
adehecq
reviewed
Mar 19, 2024
adehecq
approved these changes
Mar 19, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart some minor comments, looking good to me. Thanks !!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR improves
Raster.to_pointcloud()
in terms of argument consistency, modularity, and makes its subsampling functionality only returns valid values (while before it returned any random sample) to also be consistent with other subsampling methods.Additionally, this PR adds a non-public
Raster._load_only_mask()
function called by a new public functionRaster.get_mask()
, which allows to get the nodata mask without loading the array into memory. This is used in.to_pointcloud()
to perform sampling of only valid values without loading the array in memory 😄.The PR also adds a lot of tests!
Detailed changes
This PR:
Raster.to_points()
intoRaster.to_pointcloud()
for consistent naming in preparation for AddPointCloud
subclass ofVector
#492,data_column_name
argument forRaster.to_pointcloud()
to associate the point cloud to a certaindata_band
(instead of having a hard-coded "b1", "b2", etc), and proposes to store all columns using the new argumentstore_auxiliary_bands
(naming to be made consistent withPointCloud
class but should be fairly close),random_state
argument forsubsample
,subsample_array()
, after calling a new_load_only_mask
function to get the valid mask from disk without loading the raster array,_load_rio
to allow to return only the mask by callingrasterio.read_masks()
,Raster.get_mask()
function that returns the raster mask consistently as a boolean array (to circumvent the issue of having a singleFalse
value, see Deal consistently withRaster.data.mask=False
#505) and without loading the raster array.Resolves #499
Resolves #504
Resolves #505
Resolves #502
TO-DO:
subsample=1
insubsample_array()
,_load_only_mask
,get_mask
that works without the array? See Deal consistently withRaster.data.mask=False
#505.