Conversation
|
@paleolimbot does this look more reasonable? |
paleolimbot
left a comment
paleolimbot
left a comment
There was a problem hiding this comment.
Thank you! This is great...I am picky about these things but this is much better and I can file a follow up if you'd prefer to move on 🙂
| if err := json.Unmarshal(meta.CRS, &crsStr); err == nil { | ||
| if strings.HasPrefix(crsStr, "EPSG:") { | ||
| if code, err := strconv.Atoi(crsStr[5:]); err == nil { | ||
| return code | ||
| return code, meta.Edges | ||
| } | ||
| } | ||
| return 0 | ||
| return 0, meta.Edges | ||
| } |
There was a problem hiding this comment.
The other one that will show up here that is identical to "EPSG:4326" is "OGC:CRS84" (I wish this weren't true for what it's worth 😬 ). You can return an SRID of 4326 for this case.
| if err := json.Unmarshal(meta.CRS, &crs); err == nil { | ||
| if strings.EqualFold(crs.ID.Authority, "EPSG") && crs.ID.Code != 0 { | ||
| return crs.ID.Code | ||
| return crs.ID.Code, meta.Edges | ||
| } | ||
| } |
There was a problem hiding this comment.
The other one that will show up here is {"authority":"OGC","code":"CRS84"}. In Arrow C++ we also handle {"authority":"EPSG","code":"4326"} (where 4326 is a string) but I've never seen that in the wild.
| if srid == 4326 && edges == "spherical" { | ||
| return "geography" | ||
| } | ||
| return "geography" | ||
| return "geometry" |
There was a problem hiding this comment.
I am a geospatial snob and so I probably would do:
if srid == 4326 && edges == "spherical" {
return "geography"
} else if srid == 4326 && edges == "spherical" {
// error: force user-specified spatial type from the option because snowflake geography
// only supports SRID 4326, so both geography and geometry are incorrect in different ways
} else {
return "geometry"
}...but your way is fine for all practical purposes (as long as you handle OGC:CRS84 below).
There was a problem hiding this comment.
I adjusted this and added more tests!
Co-authored-by: Dewey Dunnington <dewey@dunnington.ca>
2 participants
No description provided.