Conversation
- Fix Scala compiler warnings: remove redundant import in Moran.scala, unreachable code in GeoParquetMetaData.scala, add missing match cases in BroadcastIndexJoinExec.scala and JoinQueryDetector.scala, fix uninitialized variable order in ScalaExample.scala - Replace System.out.println with SLF4J/Log4j loggers across test and main source files to eliminate console noise during builds - Raise test log4j2 root logger level from info to warn across all modules (common, spark, flink)
There was a problem hiding this comment.
Pull request overview
This PR reduces Maven build console noise by routing ad-hoc System.out output through logging frameworks and by raising test log levels, while also addressing several Scala compiler warnings (mostly around exhaustiveness/unreachable code).
Changes:
- Raise test Log4j2 root logger level from
infotowarnacross Spark/Flink/common modules. - Replace
System.out.println/printfdebug prints with SLF4J/Log4j logging across tests and example/demo code. - Fix Scala compiler warnings (e.g., missing match cases, redundant import, unreachable code cleanup).
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| spark/spark-4.1/src/test/resources/log4j2.properties | Raise test root logging level to warn. |
| spark/spark-4.0/src/test/resources/log4j2.properties | Raise test root logging level to warn. |
| spark/spark-3.5/src/test/resources/log4j2.properties | Raise test root logging level to warn. |
| spark/spark-3.4/src/test/resources/log4j2.properties | Raise test root logging level to warn. |
| spark/common/src/test/resources/log4j2.properties | Raise test root logging level to warn. |
| spark/common/src/test/java/org/apache/sedona/viz/NYCTripPointMapper.java | Replace stdout printing with SLF4J logging in test helper. |
| spark/common/src/test/java/org/apache/sedona/core/spatialPartitioning/quadtree/RenderQuadTree.java | Replace stdout printing with SLF4J logging in test UI helper. |
| spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/strategy/join/JoinQueryDetector.scala | Add missing match cases to remove Scala exhaustiveness warnings. |
| spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/strategy/join/BroadcastIndexJoinExec.scala | Add missing match case to remove Scala warnings in plan string rendering. |
| spark/common/src/main/scala/org/apache/spark/sql/execution/datasources/geoparquet/GeoParquetMetaData.scala | Simplify mapField logic to avoid Scala warnings. |
| spark/common/src/main/scala/org/apache/sedona/viz/showcase/ScalaExample.scala | Reorder initialization and reduce direct stdout noise (still uses println). |
| spark/common/src/main/scala/org/apache/sedona/stats/autocorrelation/Moran.scala | Remove redundant import to fix Scala warning. |
| spark/common/src/main/scala/org/apache/sedona/core/showcase/ScalaExample.scala | Replace System.out.println with println. |
| spark/common/src/main/scala/org/apache/sedona/core/showcase/ScalaEarthdataMapperRunnableExample.scala | Replace System.out.println with println. |
| spark/common/src/main/java/org/apache/sedona/viz/showcase/Example.java | Replace stdout printing with Log4j logging in demo. |
| spark/common/src/main/java/org/apache/sedona/viz/extension/visualizationEffect/ChoroplethMap.java | Replace stdout printing with existing logger calls. |
| spark/common/src/main/java/org/apache/sedona/viz/core/ImageSerializableWrapper.java | Replace stdout printing with Log4j warning. |
| spark/common/src/main/java/org/apache/sedona/core/showcase/Example.java | Replace stdout printing with Log4j logging in demo. |
| spark/common/src/main/java/org/apache/sedona/core/showcase/EarthdataMapperRunnableExample.java | Replace stdout printing with Log4j logging in demo. |
| spark/common/src/main/java/org/apache/sedona/core/formatMapper/shapefileParser/ShapefileRDD.java | Replace stdout printing with SLF4J logging. |
| snowflake/src/main/java/org/apache/sedona/snowflake/snowsql/ddl/DDLGenerator.java | Replace stdout printing with SLF4J logging for CLI usage + DDL output. |
| snowflake-tester/src/test/java/org/apache/sedona/snowflake/snowsql/TestBase.java | Replace stdout printing with test logger output. |
| snowflake-tester/src/test/java/org/apache/sedona/snowflake/snowsql/SnowTestRunner.java | Replace stdout printing with SLF4J logging. |
| snowflake-tester/src/test/java/org/apache/sedona/snowflake/snowsql/SnowClient.java | Replace stdout printing with structured logger output. |
| flink/src/test/resources/log4j2.properties | Raise test root logging level to warn. |
| flink/src/test/java/org/apache/sedona/flink/FunctionTest.java | Replace stdout printing with SLF4J debug logging. |
| common/src/test/resources/log4j2.properties | Raise test root logging level to warn. |
| common/src/test/java/org/apache/sedona/common/geometrySerde/SpatialIndexSerdeTest.java | Replace stdout printing with SLF4J logging in tests. |
| common/src/test/java/org/apache/sedona/common/S2Geography/TestHelper.java | Replace stdout printing with SLF4J debug logging. |
| common/src/test/java/org/apache/sedona/common/FunctionsTest.java | Replace stdout printing with SLF4J debug logging. |
| common/src/test/java/org/apache/sedona/common/FunctionsProj4PerformanceTest.java | Replace stdout/printf with SLF4J logging in performance tests. |
Comments suppressed due to low confidence (1)
spark/common/src/main/scala/org/apache/sedona/viz/showcase/ScalaExample.scala:52
ConfFileis aFileInputStreamthat is repeatedly reassigned and passed toprop.load(...)but never closed. This can leak file descriptors in long-running demo runs; consider using a scoped resource pattern (e.g.,scala.util.Using) or explicitly closing the stream after eachprop.load.
val resourcePath = "/../spark/common/src/test/resources/"
var ConfFile = new FileInputStream(resourcePath + "babylon.point.properties")
val demoOutputPath = "target/demo"
val scatterPlotOutputPath =
System.getProperty("user.dir") + "/" + demoOutputPath + "/scatterplot"
prop.load(ConfFile)
val heatMapOutputPath = System.getProperty("user.dir") + "/" + demoOutputPath + "/heatmap"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
snowflake/src/main/java/org/apache/sedona/snowflake/snowsql/ddl/DDLGenerator.java
Show resolved
Hide resolved
snowflake/src/main/java/org/apache/sedona/snowflake/snowsql/ddl/DDLGenerator.java
Show resolved
Hide resolved
snowflake/src/main/java/org/apache/sedona/snowflake/snowsql/ddl/DDLGenerator.java
Show resolved
Hide resolved
snowflake/src/main/java/org/apache/sedona/snowflake/snowsql/ddl/DDLGenerator.java
Outdated
Show resolved
Hide resolved
- Revert DDL SQL output lines back to System.out.println since the tool is used with stdout redirection (java -jar ... > sedona-snowflake.sql) - Replace assert with explicit check for required geotools-version arg (assertions are disabled by default without -ea) - Fix usage text: --h -> -h to match the actual argument parser
The snowflake DDLGenerator is a CLI tool that outputs SQL to stdout for file redirection. All println usage in snowflake and snowflake-tester should remain unchanged.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
common/src/test/java/org/apache/sedona/common/S2Geography/TestHelper.java
Show resolved
Hide resolved
spark/common/src/main/scala/org/apache/sedona/viz/showcase/ScalaExample.scala
Show resolved
Hide resolved
spark/common/src/main/scala/org/apache/sedona/core/showcase/ScalaExample.scala
Show resolved
Hide resolved
...mon/src/main/scala/org/apache/sedona/core/showcase/ScalaEarthdataMapperRunnableExample.scala
Show resolved
Hide resolved
compareTo(geoWKT, geoWKT) was comparing the same object, making the assertion always pass. Changed to compareTo(geoWKB, geoWKT).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...on/src/main/scala/org/apache/spark/sql/sedona_sql/strategy/join/BroadcastIndexJoinExec.scala
Show resolved
Hide resolved
.../common/src/main/scala/org/apache/spark/sql/sedona_sql/strategy/join/JoinQueryDetector.scala
Show resolved
Hide resolved
Replace placeholder match arms with explicit exceptions: - Raster + distance: createStreamShapes deserializes as geometry, would fail on raster bytes - Geography + non-raster: not yet implemented - Geography + raster: not yet implemented
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
common/src/test/java/org/apache/sedona/common/FunctionsProj4PerformanceTest.java
Show resolved
Hide resolved
common/src/test/java/org/apache/sedona/common/FunctionsProj4PerformanceTest.java
Show resolved
Hide resolved
...k/common/src/main/java/org/apache/sedona/core/formatMapper/shapefileParser/ShapefileRDD.java
Show resolved
Hide resolved
common/src/test/java/org/apache/sedona/common/FunctionsProj4PerformanceTest.java
Show resolved
Hide resolved
The compareTo(geoWKT, geoWKT) bug masks a deeper issue: WKB and WKT readers produce incompatible Geography types for empty geometries, causing compareTo to fail. Reverting to unblock CI; tracked in a separate issue.
Summary
System.out.printlnwith SLF4J/Log4j loggers across test and main source files to eliminate console noise during Maven buildsinfotowarnacross all modules (common, spark, flink)Test plan
Closes #2744