Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Docs and CI/CD #5

Merged
merged 14 commits into from
Apr 29, 2024
Merged

Docs and CI/CD #5

merged 14 commits into from
Apr 29, 2024

Conversation

BenCurran98
Copy link
Collaborator

@BenCurran98 BenCurran98 commented Apr 22, 2024

You can view a preview of the documentation here

@BenCurran98 BenCurran98 changed the title Add initial github actions Docs and CI/CD Apr 22, 2024
@BenCurran98 BenCurran98 force-pushed the DocsAndCICD branch 2 times, most recently from 2f4c89b to 1db23ba Compare April 22, 2024 23:15
Signed-off-by: BenCurran98 <b.curran@fugro.com>
Signed-off-by: BenCurran98 <b.curran@fugro.com>
Signed-off-by: BenCurran98 <b.curran@fugro.com>
Signed-off-by: BenCurran98 <b.curran@fugro.com>
Signed-off-by: BenCurran98 <b.curran@fugro.com>
Signed-off-by: BenCurran98 <b.curran@fugro.com>
Signed-off-by: BenCurran98 <b.curran@fugro.com>
Signed-off-by: BenCurran98 <b.curran@fugro.com>
Signed-off-by: BenCurran98 <b.curran@fugro.com>
Signed-off-by: BenCurran98 <b.curran@fugro.com>
Signed-off-by: BenCurran98 <b.curran@fugro.com>
Signed-off-by: BenCurran98 <b.curran@fugro.com>
@BenCurran98 BenCurran98 marked this pull request as ready for review April 23, 2024 06:46
las = load_las("example.las")

# only extract position and classification
las = load_las("example", [:position, :classification])
Copy link
Collaborator

Choose a reason for hiding this comment

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

should not this be example.las/z?

```

## Reading
To read the entire contents of a *LAS* or *LAZ* file, you can use the `load_las` function. This return a `LasDataset` with all the quantities listed above. You also have the option of only loading certain point fields.
Copy link
Collaborator

@i-kieu i-kieu Apr 23, 2024

Choose a reason for hiding this comment

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

Second sentence:
"This returns a [...]"
Also "the quantities" are returned? Do you mean to say qualities?

```

## Writing
You can write the contents of your `LasDataset` to a file by using the `save_las` function. Note that this takes either a `LasDataset` as a whole, or a tabular point cloud, *(E)VLRs* and user-defined bytes separately.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The note reads funny.
"Note that this takes either a LasDataset as a whole, or as a tabular point cloud, with (E)VLRs and user-defined bytes [something missing here] separately."


## Data Consistency

When creating a `LasDataset` or writing a tabular point cloud out to a file, we need to make sure that the header information we provide is consistent with that of the point cloud and any *VLRs* and user bytes. Internally, this is done usinjg the function `make_consistent_header!`, which compares a `LasHeader` and some *LAS* data and makes sure the header has the appropriate data offsets, flags and other metadata. This will, for example, make sure that the numbers of points, *VLRs* and *EVLRs* are consistent with the data we've provided, so your `LasDataset` is guaranteed to be consistent.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wish you would do the one-sentence-per-line-of-code thing 😆

Typo in second sentence fifth word, "using".

LAS.FullRecord
```

This was done largely to increase performance of reading point records, since having one single type for point records would require more conditional checks to see if certain extra fields need to be read from a file which ends up congesting the read process. Instead, we use *Julia*'s multiple dispatch and define `Base.read` and `Base.write` methods for each record type and avoid these checks and also decrease the type inference time when reading these into a vector.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Slayed.

Comment on lines 75 to 77
* How many bytes the point format is
* How many user fields in this record and their data size in bytes
* How many undocumented bytes there are
Copy link
Collaborator

Choose a reason for hiding this comment

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

Grammatically since this is a sentence leading into a list of dot-points:
"""

  • How many bytes the point format is,
  • How many user fields there are in the record and their data size in bytes, and
  • How many undocumented bytes there are.

"""


Firstly, the *LAS* 1.4 spec officially supports the following data types directly: `UInt8`, `Int8`, `UInt16`, `Int16`, `UInt32`, `Int32`, `UInt64`, `Int64`, `Float32` and `Float64`

This means that every `ExtraBytes` *VLR* **must** be have a data type among these values (note that vectors are not directly supported). *LAS.jl* supports static vectors (static sizing is essential) as user fields as well by internally separating out vector components and adding an `ExtraBytes` *VLR* for each component following the naming convention in the spec. That is, for a user field with `N` entries, the individual component names that are documented in the *VLRs* are "col [0]", "col [1]", ..., "col [N - 1]".
Copy link
Collaborator

Choose a reason for hiding this comment

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

First sentence eighth word "be" does not need to be there.


This means that every `ExtraBytes` *VLR* **must** be have a data type among these values (note that vectors are not directly supported). *LAS.jl* supports static vectors (static sizing is essential) as user fields as well by internally separating out vector components and adding an `ExtraBytes` *VLR* for each component following the naming convention in the spec. That is, for a user field with `N` entries, the individual component names that are documented in the *VLRs* are "col [0]", "col [1]", ..., "col [N - 1]".

When a user passes a custom field to the system, it will firstly check that the data type for this field is either one of the above types or an `SVector` of one. If it is a vector, it will construct a list of the component element field naems as above. Then, it will extract all `ExtraBytes` *VLRs* and check if any of them have matching names and update them iff they exist so their data type matches the new type supplied. If these are new fields, a new `ExtraBytes` *VLR* will be added per field name. Finally, the header is updated to reflect the new number of *VLRs*, the new data offstes and the new point record lengths.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Second sentence third last word typo, "names".
Third sentence I would recommend typing out iff to if and only if.
Last sentence seventh last word typo, "offsets".

las = load_las("my_pc.las", [:position, :classification, :thing])
```

It's also worth mentioning too that you can save "undocumented bytes" to any point as well by labelling them with the column `:undocumented_bytes`. In practice, it is possible to either have point records with user fields and undocumented bytes, just one or the other, or neither (just the *LAS* point itself).
Copy link
Collaborator

@i-kieu i-kieu Apr 23, 2024

Choose a reason for hiding this comment

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

Don't need the "too" after "It's also worth mentioning", and you can probably remove the "as well" after the "to any point".
Second sentence might read more clearly without the "either".

docs/src/vlrs.md Outdated
OGC_WKT
```

One benefit of using the *OGC WKT* is that you can specify what units of measurement are used for your point coordinates along both the *XY* plane and the *Z* axis. When reading a *LAS* file, the system can detect if an *OGC WKT* is present and will, if requested by the user, convert the point coordinates to metres.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe I am being picky with this one but
"point coordinates both in the XY plane and along the Z axis."

docs/src/vlrs.md Outdated
TextAreaDescription
```

Using the dataset `las` above, we can add a description as follows (and save/read it as we did above). Note you can also rrepeat the way the Classification Lookup was saved above too.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo second sentence, "repeat".

docs/src/vlrs.md Outdated

### Extra Bytes

Extra Bytes *VLRs* are a type of *VLR* that documents any user fields that have been added to point records in your *LAS* data. You can find an in-depth explaination of how to save/load user defined fields to your points [here](./user_fields.md).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo second sentence, "explanation".

src/header.jl Outdated
"""
$(TYPEDSIGNATURES)

Set the spatial information associate to points in a LAS file with a header `header`
Copy link
Collaborator

Choose a reason for hiding this comment

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

"associated"

Signed-off-by: BenCurran98 <b.curran@fugro.com>
Signed-off-by: BenCurran98 <b.curran@fugro.com>
$(TYPEDSIGNATURES)

Get the LAS specification version from a header `h`
"""
las_version(h::LasHeader) = h.las_version
Copy link
Collaborator

Choose a reason for hiding this comment

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

image

It doesn't work in this case as we are doing a few other manipulations on some of the other fields... but if we're going to make functions on accessing all the fields, might be worth making something like this in utils / somewhere... or making this into a macro...

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh wait.. if you change the fieldname it would change... lol

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah it's tricky do do in a non-manual, nice way

@BenCurran98 BenCurran98 merged commit 190a6b4 into main Apr 29, 2024
9 checks passed
@BenCurran98 BenCurran98 deleted the DocsAndCICD branch April 29, 2024 00:01
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants