paleolimbot commented on code in PR #749:
URL: https://github.com/apache/sedona-db/pull/749#discussion_r3239210482
##########
rust/sedona-raster/src/array.rs:
##########
@@ -230,11 +233,16 @@ impl<'a> RasterRef for RasterRefImpl<'a> {
// Only the canonical identity view (null view row) is written today.
// A non-null view row would require the view → byte-stride composition
- // path that is deferred to a follow-up; reject it here so callers see
- // a clean "no band" rather than a panic.
- if !self.band_view_list.is_null(band_row) {
- return None;
- }
+ // path, which is not yet implemented. Surface it loudly here rather
+ // than silently returning None, so callers see a clear invariant
+ // violation instead of an out-of-range-looking miss.
+ assert!(
+ self.band_view_list.is_null(band_row),
+ "{}",
+ sedona_common::sedona_internal_datafusion_err!(
+ "non-null view row at band {band_row}: view composition is not
yet implemented"
+ )
+ );
Review Comment:
An Err is definitely better (particularly if it already was that way) but
the internal err within the assert is the particularly strange bit. If you need
to make this an option you can do:
```rust
if !self.band_view_list.is_null(band_row) {
painic!(
"non-null view row at band {band_row}: view composition is
not yet implemented"
);
}
```
(but Python release builds will just crash on a panic without a user facing
error)
--
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]
