paleolimbot commented on code in PR #1055:
URL: https://github.com/apache/sedona-db/pull/1055#discussion_r3603531974


##########
python/sedonadb/tests/functions/test_rs_clip.py:
##########
@@ -42,7 +42,8 @@
 )
 
 pytest.importorskip("rasterio")
-pytest.importorskip("shapely")
+shapely = pytest.importorskip("shapely")

Review Comment:
   shapely is one of our test dependencies (maybe not explicitly but could be 
since geopandas depends on it)...a regular import should be fine here



##########
rust/sedona-raster-gdal/src/rs_clip.rs:
##########
@@ -1432,4 +1456,105 @@ mod tests {
             "all_touched=false should hint at the sub-pixel case: {msg}"
         );
     }
+
+    #[test]
+    fn test_rs_clip_band_data_is_block_backed() {
+        // The clipped band bytes move into the output as a zero-copy view
+        // block; pin that so a refactor can't silently reintroduce the copy.
+        // (Views at or under the inline threshold store their bytes inline,
+        // so the band must be bigger than that.)
+        let values: Vec<u8> = (0..32).collect();
+        let array = raster_array([Some(RasterSpec::d2(8, 
4).crs(None).band_values(&values))]);
+
+        let kernel = RsClip { arg_count: 3 };
+        let result = kernel
+            .invoke_batch(

Review Comment:
   I think the tester would work here unless there's something specific to what 
you're testing (would collapse some of these literal values into more compact 
representations like `1` and `"the wkt"`)



##########
python/sedonadb/tests/functions/test_rs_clip.py:
##########
@@ -42,7 +42,8 @@
 )
 
 pytest.importorskip("rasterio")

Review Comment:
   Should this importorskip be scoped to a test rather than a module?



-- 
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