jesspav commented on code in PR #360:
URL: https://github.com/apache/sedona-db/pull/360#discussion_r2566187398
##########
c/sedona-proj/src/st_transform.rs:
##########
@@ -293,7 +293,7 @@ fn invoke_scalar(wkb: &Wkb, trans: &dyn CrsTransform,
builder: &mut BinaryBuilde
fn parse_source_crs(source_type: &SedonaType) -> Result<Option<String>> {
match source_type {
SedonaType::Wkb(_, Some(crs)) | SedonaType::WkbView(_, Some(crs)) => {
- crs.to_authority_code()
+ Ok(Some(crs.to_crs_string()))
Review Comment:
In `st_transform` it would have err'd with `Source CRS is required when
transforming to a CRS`, unless somebody had explicitly specified lenient. in
either case it's a good fix. thanks.
##########
rust/sedona-schema/src/crs.rs:
##########
@@ -97,10 +97,37 @@ impl PartialEq<dyn CoordinateReferenceSystem + Send + Sync>
/// A trait defining the minimum required properties of a concrete coordinate
/// reference system, allowing the details of this to be implemented elsewhere.
pub trait CoordinateReferenceSystem: Debug {
+ /// Compute the representation of this Crs in the form required for JSON
output
+ ///
+ /// The output must be valid JSON (e.g., arbitrary strings must be quoted).
fn to_json(&self) -> String;
+
+ /// Compute the representation of this Crs as a string in the form
Authority:Code
+ ///
+ /// If there is no such representation, returns None.
fn to_authority_code(&self) -> Result<Option<String>>;
+
+ /// Compute CRS equality
+ ///
+ /// CRS equality is a relatively thorny topic and can be difficult to
compute;
+ /// however, this method should try to compare self and other on value
(e.g.,
+ /// comparing authority_code where possible).
fn crs_equals(&self, other: &dyn CoordinateReferenceSystem) -> bool;
+
+ /// Reduce this beautiful, rich CRS representation to a mere integer if
possible
+ ///
+ /// For the purposes of this trait, an SRID is always equivalent to the
+ /// authority_code `"EPSG<srid>"`. Note that other SRID representations
+ /// (e.g., GeoArrow, Parquet GEOMETRY/GEOGRAPHY) do not make any guarantees
+ /// that an SRID comes from the EPSG authority.
fn srid(&self) -> Result<Option<u32>>;
+
+ /// Compute a CRS string representation
+ ///
+ /// Unlike `to_json()`, arbitrary string values returned by this method
should
+ /// not be escaped. This is the representation expected as input to PROJ,
GDAL,
+ /// and Parquet GEOMETRY/GEOGRAPHY representations of CRS.
+ fn to_crs_string(&self) -> String;
Review Comment:
Thanks for adding this, those escapes were annoying. :)
--
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]
