jiayuasu commented on code in PR #3133:
URL: https://github.com/apache/sedona/pull/3133#discussion_r3620080410
##########
spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/io/netcdfmetadata/NetCdfMetadataPartitionReader.scala:
##########
@@ -569,11 +571,79 @@ class NetCdfMetadataPartitionReader(
.orElse(Option(globalAttrs.findAttributeString("crs_wkt", null)))
.orElse(Option(globalAttrs.findAttributeString("spatial_ref", null)))
.orNull
- val srid =
- if (!needSrid) None
- else if (wkt != null) lookupSrid(wkt)
- else gmVar.flatMap(latitudeLongitudeSrid)
- (wkt, srid)
+ if (wkt != null) {
+ (wkt, if (needSrid) lookupSrid(wkt) else None)
+ } else {
+ gmVar.flatMap(v => translateGridMapping(v, gridVar)) match {
+ case Some(proj) =>
+ (Try(proj.toWkt2).getOrElse(null), if (needSrid) projSrid(proj) else
None)
+ case None => (null, None)
+ }
Review Comment:
Fixed in e0cee7d22b0. Derived WKT and SRID are now produced atomically: a
null or failed WKT2 serialization returns `(null, None)`, and the lazy SRID
lookup is not evaluated on that path. Added a regression test for the throwing
serializer path; `netcdfMetadataTest` passes 61/61.
##########
spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/io/netcdfmetadata/NetCdfMetadataPartitionReader.scala:
##########
@@ -569,11 +571,79 @@ class NetCdfMetadataPartitionReader(
.orElse(Option(globalAttrs.findAttributeString("crs_wkt", null)))
.orElse(Option(globalAttrs.findAttributeString("spatial_ref", null)))
.orNull
- val srid =
- if (!needSrid) None
- else if (wkt != null) lookupSrid(wkt)
- else gmVar.flatMap(latitudeLongitudeSrid)
- (wkt, srid)
+ if (wkt != null) {
+ (wkt, if (needSrid) lookupSrid(wkt) else None)
+ } else {
+ gmVar.flatMap(v => translateGridMapping(v, gridVar)) match {
+ case Some(proj) =>
+ (Try(proj.toWkt2).getOrElse(null), if (needSrid) projSrid(proj) else
None)
+ case None => (null, None)
+ }
+ }
+ }
+
+ /**
+ * CRS defined through CF grid mapping parameters, translated to a
projection via proj4sedona
+ * (pure Java — keeps this data source free of the optional LGPL GeoTools
runtime). CF requires
+ * `false_easting`/`false_northing` to be in the units of the projection
coordinates, so those
+ * units ride along. A `latitude_longitude` mapping is translated only when
its attributes
+ * positively identify the Earth figure (a resolvable datum or ellipsoid
name, explicit figure
+ * parameters, or `towgs84`) — the WGS 84 assumption GDAL and pyproj apply
to a bare geographic
+ * mapping must not be reported as if the file declared it. Unsupported or
malformed mappings
+ * translate to nothing rather than failing the row.
+ */
+ private def translateGridMapping(gmVar: Variable, gridVar:
Option[Variable]): Option[Proj] = {
+ Option(attrString(gmVar, "grid_mapping_name")).flatMap { name =>
+ Try {
+ val attrs = cfAttributeMap(gmVar)
+ if (name.trim.equalsIgnoreCase("latitude_longitude") &&
+ !CfGridMapping.identifiesEarthShape(attrs)) {
+ null
+ } else {
+ val (xUnits, yUnits) =
+ gridVar.map(projectionCoordinateUnits).getOrElse((null, null))
+ CfGridMapping.toProj(attrs, xUnits, yUnits)
+ }
+ }.toOption.flatMap(Option(_))
+ }
+ }
+
+ /**
+ * The grid mapping variable's attributes as the value shapes proj4sedona
coerces: strings stay
+ * strings, single numerics become `Number`, and multi-valued numerics (e.g.
the two standard
+ * parallels) become `double[]`. Unsigned values are widened to their
positive representation;
+ * non-numeric array attributes are skipped rather than misread.
+ */
+ private def cfAttributeMap(gmVar: Variable): java.util.Map[String, Object] =
{
+ val map = new java.util.HashMap[String, Object]()
+ gmVar.attributes().asScala.foreach { attr =>
+ val value: Object =
+ if (attr.isString) attr.getStringValue
+ else if (attr.getLength == 1) numericAttrValue(attr, 0)
+ else {
+ val values = new Array[Double](attr.getLength)
+ val numeric = (0 until attr.getLength).forall { i =>
+ val n = numericAttrValue(attr, i)
+ if (n != null) values(i) = n.doubleValue()
+ n != null
+ }
+ if (numeric) values else null
+ }
+ if (value != null) map.put(attr.getShortName, value)
+ }
+ map
+ }
+
+ /**
+ * The `units` attributes of the grid's x and y projection coordinate
variables. CF declares
+ * `false_easting` in the x unit and `false_northing` in the y unit;
proj4sedona accepts
+ * equivalent spellings and rejects incompatible axes rather than misscaling
either offset.
+ */
+ private def projectionCoordinateUnits(gridVar: Variable): (String, String) =
{
+ val (latDim, lonDim) = trailingDims(gridVar)
+ def unitsOf(dim: Dimension): String =
+ findCoordinateVariable(gridVar, dim).map(v => attrString(v,
"units")).orNull
+ (unitsOf(lonDim), unitsOf(latDim))
}
Review Comment:
Fixed in e0cee7d22b0. Renamed the tuple to `(yDim, xDim)` and now return
`(unitsOf(xDim), unitsOf(yDim))`, preserving the existing x/y result while
matching the `trailingDims` convention.
##########
pom.xml:
##########
@@ -96,7 +96,7 @@
<scala-collection-compat.version>2.5.0</scala-collection-compat.version>
<geoglib.version>1.52</geoglib.version>
<caffeine.version>2.9.2</caffeine.version>
- <proj4sedona.version>0.1.1</proj4sedona.version>
+ <proj4sedona.version>0.1.2</proj4sedona.version>
Review Comment:
`org.datasyslab:proj4sedona:0.1.2` is now published to Maven Central. I
verified resolution with `mvn -U dependency:get` against a fresh local
repository, so no temporary repository is needed. The PR CI has also been
retriggered.
--
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]
