paleolimbot opened a new issue, #834:
URL: https://github.com/apache/sedona-db/issues/834

   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<TableSchema>)` 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_
               


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to