feat(r/sedonadb): add CRS printing for sedonadb_dataframe #475

e-kotov · 2026-01-01T19:28:45Z

Adds CRS printing to sedonadb_dataframe print method and relevant helper in rust that can be reused in other functions (but I kept it unexported for now).

library(sf)
nc <- st_read(system.file("shape/nc.shp", package = "sf"), quiet = TRUE)
nc <- st_transform(nc, 5070)
x <- as_sedonadb_dataframe()
x
# A sedonadb_dataframe: ? x 16
# Geometry: geometry (CRS: EPSG:5070)
┌─────────┬───────────┬─────────┬───┬────────────────────────────────────────────────┬───────┐
│   AREA  ┆ PERIMETER ┆  CNTY_  ┆ … ┆                    geometry                    ┆  sid  │
│ float64 ┆  float64  ┆ float64 ┆   ┆                    geometry                    ┆ int32 │
╞═════════╪═══════════╪═════════╪═══╪════════════════════════════════════════════════╪═══════╡
│   0.114 ┆     1.442 ┆  1825.0 ┆ … ┆ MULTIPOLYGON(((1288822.3256753243 1563699.442… ┆     1 │
├╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┼╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┤
│   0.061 ┆     1.231 ┆  1827.0 ┆ … ┆ MULTIPOLYGON(((1307046.8945761852 1581379.933… ┆     2 │
├╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┼╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┤
│   0.143 ┆      1.63 ┆  1828.0 ┆ … ┆ MULTIPOLYGON(((1378069.6323642111 1578869.811… ┆     3 │
├╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┼╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┤
│    0.07 ┆     2.968 ┆  1831.0 ┆ … ┆ MULTIPOLYGON(((1765394.8318386688 1660922.198… ┆     4 │
├╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┼╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┤
│   0.153 ┆     2.206 ┆  1832.0 ┆ … ┆ MULTIPOLYGON(((1661822.2229292062 1630503.661… ┆     5 │
├╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┼╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┤
│   0.097 ┆      1.67 ┆  1833.0 ┆ … ┆ MULTIPOLYGON(((1703230.114237553 1638095.6588… ┆     6 │
└─────────┴───────────┴─────────┴───┴────────────────────────────────────────────────┴───────┘
Preview of up to 6 row(s)

Co-authored-by: Copilot <[email protected]>

Copilot

Pull request overview

This pull request adds CRS (Coordinate Reference System) printing functionality to the sedonadb_dataframe print method. When printing a dataframe with geometry columns, the CRS information is now displayed below the header, showing the geometry column names along with their CRS identifiers (e.g., "EPSG:5070", "OGC:CRS84").

Key Changes:

Added Rust function parse_crs_metadata to extract CRS information from GeoArrow metadata
Enhanced print.sedonadb_dataframe to display geometry column CRS information with width-aware truncation
Created comprehensive test suite covering various CRS scenarios including EPSG codes, engineering CRS, and edge cases

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
r/sedonadb/src/rust/src/lib.rs	Implements `parse_crs_metadata` Rust function to parse CRS from GeoArrow JSON metadata
r/sedonadb/src/rust/api.h	Adds FFI declaration for the new `parse_crs_metadata` function
r/sedonadb/src/rust/Cargo.toml	Adds `serde_json` dependency for JSON parsing
r/sedonadb/src/init.c	Registers the new `parse_crs_metadata` C binding
r/sedonadb/R/000-wrappers.R	Adds R wrapper for `parse_crs_metadata` FFI function
r/sedonadb/R/crs.R	Introduces `sd_parse_crs` helper function for parsing CRS metadata
r/sedonadb/R/dataframe.R	Enhances `print.sedonadb_dataframe` to display CRS information for geometry columns with truncation support
r/sedonadb/tests/testthat/test-crs.R	Adds comprehensive tests for CRS parsing and display functionality

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

r/sedonadb/tests/testthat/test-crs.R

r/sedonadb/src/rust/src/lib.rs

r/sedonadb/R/crs.R

r/sedonadb/tests/testthat/test-crs.R

r/sedonadb/R/dataframe.R

paleolimbot

Thank you for working on this! I love the column count, CRS, and geometry column information output when printing. I added a few high-level suggestions but with some tweaking we can merge the current approach too. The spirit of all my comments is that I'd love to use some of the places where we've already implemented some of this for Rust or Python or geoarrow/r already.

Something I added to the Python bindings but forgot to add here was a bare-bones wrapper around the SedonaDB/DataFusion schema, which provides access to column/type information including the CRS: https://github.com/apache/sedona-db/blob/e0e1d109480727faaf7be25923b57b4686144438/python/sedonadb/src/schema.rs . I added some suggestions inline about how to draw a few ideas from that hopefully without widening the scope of this PR too much 🙂

r/sedonadb/R/000-wrappers.R

r/sedonadb/R/dataframe.R

r/sedonadb/R/pkg-sf.R

r/sedonadb/src/rust/src/lib.rs

r/sedonadb/tests/testthat/test-crs.R

e-kotov · 2026-01-07T12:15:24Z

@paleolimbot I think I addressed all comments, but I might need some help with how to best approach the merge conflict.

paleolimbot

Apologies for the merge conflicts...I was trying to make it easier to develop the R package but it definitely conflicted with this PR 😬 . You should be able to git pull and use tools/update-savvy.sh and air format with the package now.

This is looking great! A few things we should solve here I think but I love the improved output and I think this is close!

r/sedonadb/src/rust/src/ffi.rs

paleolimbot · 2026-01-07T15:14:39Z

r/sedonadb/tests/testthat/test-crs.R

+  expect_snapshot(sedonadb:::sd_parse_crs(meta))
+})
+
+# Tests for CRS display in print.sedonadb_dataframe


nit: I like your other tests that put this comment inside the test_that() block (I think this is what I did for the tests in some of the other files too)

r/sedonadb/tests/testthat/test-crs.R

paleolimbot · 2026-01-07T15:31:09Z

r/sedonadb/tests/testthat/test-crs.R

+test_that("sd_parse_crs handles empty string", {
+  expect_snapshot(
+    sedonadb:::sd_parse_crs(""),
+    error = TRUE
+  )
+})


This test feels like it should be renamed (or the behaviour modified such that it handles the empty string)

paleolimbot · 2026-01-07T15:36:03Z

r/sedonadb/src/rust/src/lib.rs

+                out.set_name(0, "authority_code")?;
+                out.set_name(1, "srid")?;
+                out.set_name(2, "name")?;
+                out.set_name(3, "proj_string")?;


It might be more appropriate to call this input (which is the term sf uses to describe this concept, sort of), or maybe definition. (A "proj string" carries the connotation specific formatting that is not how this is typically formatted here)

paleolimbot · 2026-01-07T15:53:05Z

r/sedonadb/src/rust/src/lib.rs

+                        inner: crs_arc.clone(),
+                    })
+                } else {
+                    Err(savvy::Error::new("No CRS available for this geometry type"))


The docstring says this returns NULL for the "no crs" case. If this is hard to do with savvy maybe just update the docstring explaining that.

paleolimbot · 2026-01-07T15:53:29Z

r/sedonadb/src/rust/src/lib.rs

+        match self.inner.srid() {
+            Ok(Some(srid)) => savvy::Sexp::try_from(srid as i32),
+            Ok(None) => Ok(savvy::NullSexp.into()),
+            Err(e) => Err(savvy::Error::new(format!("Failed to get SRID: {e}"))),


The docstring says this should return NULL for this case?

paleolimbot · 2026-01-07T15:57:32Z

r/sedonadb/src/rust/src/lib.rs

+    /// Get a formatted CRS display string like " (CRS: EPSG:4326)" or empty string
+    fn crs_display(&self) -> savvy::Result<savvy::Sexp> {
+        use sedona_schema::datatypes::SedonaType;
+
+        match &self.inner {


Do we need this one (from R you can do sd_type$crs()$display()?)

paleolimbot · 2026-01-07T15:59:01Z

r/sedonadb/src/rust/src/lib.rs

+
+        // Use existing SedonaType infrastructure to parse the field
+        let inner = SedonaType::from_storage_field(&field)
+            .map_err(|e| savvy::Error::new(format!("Failed to create SedonaType: {e}")))?;


Suggested change

.map_err(|e| savvy::Error::new(format!("Failed to create SedonaType: {e}")))?;

.map_err(|e| savvy_err!("Failed to create SedonaType: {e}"))?;

(I've been trying to consistently use savvy_err!() elsewhere but I'm new to this so the conventions aren't perfect)

paleolimbot · 2026-01-07T16:04:39Z

r/sedonadb/tests/testthat/_snaps/crs.md

+      +-----------------------------+----------------------------+
+      +-----------------------------+----------------------------+
+      Preview of up to 0 row(s)
+


Should we add a snapshot test for printing without any geometry column?

e-kotov · 2026-01-07T16:14:36Z

Apologies for the merge conflicts...I was trying to make it easier to develop the R package

no worries, and air.toml is very welcome! I previously disabled mine locally to prevent unnecessary code changes, as I did not know your preference for this.

I will get back to you on the questions a bit later. Thanks for another thorough review!

e-kotov and others added 6 commits January 1, 2026 18:34

add crs parsing and printing

eb1afe2

Update r/sedonadb/R/crs.R

90d78fa

Co-authored-by: Copilot <[email protected]>

Update r/sedonadb/src/rust/src/lib.rs

fc7c375

Co-authored-by: Copilot <[email protected]>

Update r/sedonadb/R/dataframe.R

ee8fa5c

Co-authored-by: Copilot <[email protected]>

add crs printing tests

7e616e4

add license to crs.R

ac60e14

Copilot AI review requested due to automatic review settings January 1, 2026 19:28

Copilot started reviewing on behalf of e-kotov January 1, 2026 19:29 View session

Copilot AI reviewed Jan 1, 2026

View reviewed changes

address copilot review issues

c6ee791

paleolimbot reviewed Jan 2, 2026

View reviewed changes

e-kotov mentioned this pull request Jan 5, 2026

feat(r/sedonadb): Translate R expressions to DataFusion logical Expr #468

Merged

e-kotov added 8 commits January 7, 2026 11:06

add savvy update

17cbc50

run savvy update

26789b0

add snaps in tests for sd_parse_crs

5bfc62b

rust to parse crs instead of R

24258b7

additional tests for crs

9996def

savvy powered SedonaCrsR wrapper

c51a335

correct crs display

712d2b2

fix CI errors

905a1f9

paleolimbot added 2 commits January 7, 2026 08:52

Merge remote-tracking branch 'upstream/main' into crs-print

aa708e0

regenerate and format

21507be

paleolimbot reviewed Jan 7, 2026

View reviewed changes

e-kotov added 2 commits January 7, 2026 18:10

remove redundant import_arrow_field

a1de706

remove redunant ::: in tests

53a9d10

	.map_err(\|e\| savvy::Error::new(format!("Failed to create SedonaType: {e}")))?;
	.map_err(\|e\| savvy_err!("Failed to create SedonaType: {e}"))?;

feat(r/sedonadb): add CRS printing for sedonadb_dataframe #475

Are you sure you want to change the base?

feat(r/sedonadb): add CRS printing for sedonadb_dataframe #475

Uh oh!

Conversation

e-kotov commented Jan 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

e-kotov commented Jan 7, 2026

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

e-kotov commented Jan 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

e-kotov commented Jan 1, 2026 •

edited

Loading