BlakeOrth commented on code in PR #8801:
URL: https://github.com/apache/arrow-rs/pull/8801#discussion_r2505728889
##########
parquet-geospatial/src/crs.rs:
##########
@@ -0,0 +1,97 @@
+use std::{collections::HashMap, sync::Arc};
+
+use arrow_schema::{Schema, SchemaBuilder};
+use serde_json::{Value, json};
+
+#[derive(Debug)]
+pub enum Crs {
Review Comment:
> I think my point was that the current implementation (which I know is in
the works) doesn't quite do that (e.g., there are some common that currently go
through `Other(String)` where the string is actually PROJJSON), so perhaps it
would help reduce the scope of the `Crs` to make it simpler.
Yes, I think this is a fair point, especially considering your more detailed
description of the landscape around CRS metadata above. Based on what I've
gathered from this discussion at large is that the `Crs` type here is perhaps
over-constrained. I think my general thought with the type written here was
more that in cases where we can very confidently say it's PROJJSON or an SRID
we can present it as such, and the `Other(String)` cases would need to be
handled case-by-case at a higher level.
There's probably quite a bit of wisdom in keeping the underlying type
simple, and just presenting the CRS as it exists in the file and putting the
onus on the user to do with it what they will.
--
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]
