rust/sedona-pointcloud: Improve integration test coverage for las and laz readers
- Dominant language
- Rust
- Stars
- 503
- Forks
- 61
- Avg merge
- 2d 4h
- Merged PRs (30d)
- 90
Description
A couple of suggestions before merging:
**Tests**
- The previous `projection` test covered both `.las` and `.laz`; the rewrite only covers `.las`. Since LAZ goes through a separate decode path, it'd be good to keep the `.laz` assertion (or parameterize over both).
- Assertions check column count + type of column 0, but not the actual data. The interesting failure mode in #824 was returning the **right schema with the wrong column** — same UInt8 shape, wrong values. An `assert_eq!` on a value (e.g. `classification = 0` for the single point in `extra.las`) would lock that down.
**Scope**
- `LasSource::new(extension, table_schema: impl Into)` is an unrelated public-API change. Worth pulling out into a separate PR (or dropping) so this one is purely a fix.
- The field reordering in `LasSource` and `LasOpener` is noisy in the diff without changing behaviour; reverting it would make the fix easier to cherry-pick.
_Originally posted by @jiayuasu in https://github.com/apache/sedona-db/issues/825#issuecomment-4417226182_
Contributor guide
Research direction
Start with the rewritten projection integration test and the LasSource and LasOpener readers. Run the test with both extra.las and its .laz counterpart, then add an assertion for the returned classification value and keep the unrelated public-API and field-order changes out of scope.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- database, testing
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100