High-level I/O Interface #3

BenCurran98 · 2024-04-17T05:50:23Z

Adding in methods for reading and writing in a user-friendly way.
The main wrapper around a complete LAS dataset is the LasDataset struct, which contains the LAS point data (as a table), an extra mutable table of user fields kept alongside this, as well as the header, VLR, EVLR and user-defined bytes
You can load your entire LAS dataset using load_las (which returns a LasDataset) or you can just get the tabular point cloud using load_pointcloud.
To save, you can call out to save_las and enter either a LasDataset or TypedTable

There are a few methods to manipulate your LasDataset as well, such as adding VLRs and user-defined fields to your points

For performance reasons, I've split up the idea of a LAS point record into 4 structs implementing an abstract type LasRecord. Each of these have a different combination of a point, user-defined fields and undocumented extra bytes and their read/write methods take these into account. This turns out to be way faster than if you had one struct wrapping all 3 fields and doing a bunch of if-statements to check if certain fields are populated. Note also that LasRecords aren't designed to be dealt with directly by the user- the user should interact with the data either through a LasContent or TypedTable

Signed-off-by: BenCurran98 <b.curran@fugro.com>

MeganDawson42 · 2024-04-17T23:44:01Z

src/las_content.jl

+        all(indexin(evlrsA, evlrsB) .!= nothing),
+        all(indexin(evlrsB, evlrsA) .!= nothing)
+    ])
+    user_bytes_equal = (get_user_defined_bytes(contA) == get_user_defined_bytes(contB))


_user_data doesn't need to be the same?

It's implicitly checked when we check the contents of the original pointcloud input. I've restricted the inputs so users can't actually supply their own _user_data and it gets extracted from the point cloud itself

MeganDawson42 · 2024-04-17T23:47:38Z

src/dataset.jl

+"""
+function add_column!(las::LasDataset, name::Symbol, values::AbstractVector{T}) where T
+    if ismissing(las._user_data)
+        las._user_data = FlexTable(NamedTuple{ (name,) }( (values,) ))


do we need to check the length matches the las data? also we are using the ordering to keep in sync (not ids)?

Signed-off-by: BenCurran98 <b.curran@fugro.com>

i-kieu · 2024-04-19T00:04:01Z

src/dataset.jl

+"""
+    $(TYPEDSIGNATURES)
+
+Add a column with a `column` and set of `values` to a `las` dataset


This reads strangely -- "Add a column with a column ..."? Maybe "Add a column with name column and ..." is more clear.

Signed-off-by: BenCurran98 <b.curran@fugro.com>

i-kieu · 2024-04-19T00:17:53Z

src/util.jl

@@ -44,8 +61,59 @@ function open_las(func::Function, file::String, rw::String = "r")
    end
 end

+function open_laz(func, file::String, rw::String="r")


Can we do here typing of func to func::Function?

Also spacing in this function open_laz is inconsistent with other functions, e.g. line 69
if (rw=="r")
when usually we have
if rw == "r"

i-kieu · 2024-04-19T00:23:47Z

src/util.jl

+        left_brack_idx[1] < right_brack_idx[1],
+        all(isdigit, str_name[(left_brack_idx[1] + 1):(right_brack_idx[1] - 1)])
+    ]) "Invalid column name $(field)"
+    # if it's split, get the bit before the brackets, i.e for "col [1]" you get "col"


Referring to 'bit' here might cause a little confusion as you are not getting a binary bit, you're referring to the part before the bracketing.

Also as an aside, left and right brack is very funny sounding.

Signed-off-by: BenCurran98 <b.curran@fugro.com>

i-kieu · 2024-04-19T00:54:13Z

src/records.jl


-function byte_size(record::Type{TRecord}) where TRecord <: LasRecord{TPoint,N} where {N, TPoint <: LasPoint{F}} where F
-    return byte_size(TPoint) + N
+Get the appropriate LAS record format for a LAS `header` and a (possibly empty) est of `extra_bytes` that document any optional user fields to include


Spelling mistake:
"Get the appropriate LAS record format for a LAS header and a (possibly empty) set ..."

i-kieu · 2024-04-19T00:57:13Z

src/records.jl

+    record_length = point_record_length(header)
+    num_point_bytes = byte_size(point_type)
+    record_diff = record_length - num_point_bytes
+    @assert record_diff ≥ 0 "Record length in header $(record_length)B smaller than point size $(num_point_bytes)B"


Assert message could be clearer.
"Record length in header [...] is smaller than [...]"
Or perhaps more clearly
"Record length in header [...] cannot be smaller than point size [...]"

i-kieu · 2024-04-19T00:59:40Z

src/records.jl

+    user_field_types = data_type.(extra_bytes)
+    num_user_bytes = sum(sizeof.(user_field_types))
+    num_undoc_bytes = record_length - num_point_bytes - num_user_bytes
+    @assert num_undoc_bytes ≥ 0 "Record length in header $(record_length)B inconsistent with size of point $(num_point_bytes)B and size of user fields $(num_user_bytes)B"


"Record length of [...] in header is inconsistent with both the size of point [...] and of user fields [...]"

i-kieu · 2024-04-19T01:05:07Z

test/dataset.jl

+    @test other_las == las
+
+    # now try incorporating some user fields
+    spicey_pc = Table(pc, thing = rand(num_points), other_thing = rand(Int16, num_points))


Spicy is not spelled with an 'e'

It totally is

i-kieu · 2024-04-19T01:13:38Z

test/file_io.jl

+end
+
+@testset "Extra Bytes I/O" begin
+    # simple example where we have 4 extra undocumeented bytes per point


"undocumented"

Signed-off-by: BenCurran98 <b.curran@fugro.com>

BenCurran98 added 5 commits April 17, 2024 11:25

Update handling of LAS records

11d1ccf

Signed-off-by: BenCurran98 <b.curran@fugro.com>

LAS content wrapper

92c1c7c

Signed-off-by: BenCurran98 <b.curran@fugro.com>

Extra Bytes VLR

5a01e67

Signed-off-by: BenCurran98 <b.curran@fugro.com>

Add write methods

afbd034

Signed-off-by: BenCurran98 <b.curran@fugro.com>

Fix up vlr checking

45aee8f

Signed-off-by: BenCurran98 <b.curran@fugro.com>

BenCurran98 requested review from n6171028, MeganDawson42, mazadbakht, zqiaofugro and i-kieu April 17, 2024 05:50

BenCurran98 force-pushed the IO branch 2 times, most recently from 814051a to 679ad60 Compare April 17, 2024 05:51

BenCurran98 added 4 commits April 17, 2024 15:51

Add reading methods

b328327

Signed-off-by: BenCurran98 <b.curran@fugro.com>

Add some docs + fix tests

6800373

Signed-off-by: BenCurran98 <b.curran@fugro.com>

Fix tests

b6106a5

Signed-off-by: BenCurran98 <b.curran@fugro.com>

Add LAZ tests

a0c433d

Signed-off-by: BenCurran98 <b.curran@fugro.com>

BenCurran98 force-pushed the IO branch from 679ad60 to a0c433d Compare April 17, 2024 05:51

BenCurran98 added 2 commits April 18, 2024 09:39

Rename LasContent to LasDataset

08bee22

Signed-off-by: BenCurran98 <b.curran@fugro.com>

Fix file names

10a233f

Signed-off-by: BenCurran98 <b.curran@fugro.com>

MeganDawson42 reviewed Apr 17, 2024

View reviewed changes

BenCurran98 added 3 commits April 18, 2024 11:40

Add explicit tests for dataset manipulation

43297be

Signed-off-by: BenCurran98 <b.curran@fugro.com>

Make sure passing dataset with extra in and out works

a80b7ab

Signed-off-by: BenCurran98 <b.curran@fugro.com>

Add test for ExtendedRecord

74fc24d

Signed-off-by: BenCurran98 <b.curran@fugro.com>

i-kieu reviewed Apr 19, 2024

View reviewed changes

Handle case of svectors as user fields

c46184d

Signed-off-by: BenCurran98 <b.curran@fugro.com>

BenCurran98 marked this pull request as ready for review April 19, 2024 00:13

Make docs clearer

e97babd

Signed-off-by: BenCurran98 <b.curran@fugro.com>

i-kieu reviewed Apr 19, 2024

View reviewed changes

Minor touchups

8b2d4c7

Signed-off-by: BenCurran98 <b.curran@fugro.com>

i-kieu reviewed Apr 19, 2024

View reviewed changes

i-kieu approved these changes Apr 19, 2024

View reviewed changes

mazadbakht approved these changes Apr 19, 2024

View reviewed changes

BenCurran98 added 4 commits April 22, 2024 09:52

More PR feedback

43c61b9

Signed-off-by: BenCurran98 <b.curran@fugro.com>

Fix up docs

f6ef1ff

Signed-off-by: BenCurran98 <b.curran@fugro.com>

Add ability to merge fields

a56da97

Signed-off-by: BenCurran98 <b.curran@fugro.com>

Properly merge column

4222eac

Signed-off-by: BenCurran98 <b.curran@fugro.com>

MeganDawson42 approved these changes Apr 22, 2024

View reviewed changes

BenCurran98 added 6 commits April 23, 2024 08:45

Remove redundant test files

fc67d93

Signed-off-by: BenCurran98 <b.curran@fugro.com>

Fix tests

51d56da

Signed-off-by: BenCurran98 <b.curran@fugro.com>

Make convert to metres optional

a86ee45

Signed-off-by: BenCurran98 <b.curran@fugro.com>

Ensure header info is always consistent with data

15d3cd4

Signed-off-by: BenCurran98 <b.curran@fugro.com>

Use iterator to speed up reads

bcbf028

Signed-off-by: BenCurran98 <b.curran@fugro.com>

Fix issues in consistent header

98f2eb8

Signed-off-by: BenCurran98 <b.curran@fugro.com>

BenCurran98 merged commit 9affce2 into main Apr 23, 2024
1 check passed

BenCurran98 deleted the IO branch April 23, 2024 02:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

High-level I/O Interface #3

High-level I/O Interface #3

BenCurran98 commented Apr 17, 2024 •

edited

Loading

MeganDawson42 Apr 17, 2024

BenCurran98 Apr 18, 2024

MeganDawson42 Apr 17, 2024

i-kieu Apr 19, 2024

i-kieu Apr 19, 2024

i-kieu Apr 19, 2024

i-kieu Apr 19, 2024

i-kieu Apr 19, 2024

i-kieu Apr 19, 2024

i-kieu Apr 19, 2024

i-kieu Apr 19, 2024

BenCurran98 Apr 19, 2024

i-kieu Apr 19, 2024

High-level I/O Interface #3

High-level I/O Interface #3

Conversation

BenCurran98 commented Apr 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenCurran98 commented Apr 17, 2024 •

edited

Loading