zhengruifeng commented on PR #58302:
URL: https://github.com/apache/spark/pull/58302#issuecomment-5422736260
Test coverage and GraphX-fork audit
I compared this PR with the current GraphFrames checkout (`16a160b`) and
checked the OSS CI run.
Test coverage
- Every public GraphFrames algorithm exposed by this PR is executed by at
least one test.
- The Scala GraphFrames suite passed in OSS CI: **325 succeeded, 0 failed, 0
ignored**.
- The dedicated `pyspark-graphframes` OSS job passed all three registered
modules:
- `pyspark.graphframes.tests.test_graphframe`
- `pyspark.graphframes.tests.connect.test_parity_graphframe`
- `pyspark.graphframes.tests.connect.test_all_algorithms`
- Spark precompilation also passed.
This is broad algorithm coverage, but it is not a complete copy of the
upstream test tree:
- Core Scala: 28 of 34 upstream test files have counterparts. The missing
files are `SparkShimsSuite`, `BeliefPropagationSuite`, `GraphsSuite`,
`TestLDBCCases`, `PropertyGraphFrameTest`, and `KMinSamplingSuite`.
- The 18 suites for GraphFrames' private GraphX fork were intentionally not
copied because this PR uses Spark's built-in GraphX and its existing tests.
- Python upstream currently has 45 classic test functions plus 14
PropertyGraph tests. This PR has 4 classic test functions plus 10 new grouped
Connect test functions.
- `KMinSampling` has indirect coverage through consumers, but its dedicated
unit suite is absent. The Belief Propagation example and PropertyGraph APIs
were not imported.
- Therefore, having at least one test per exposed algorithm should not be
interpreted as complete backend, parameter, error-path, or upstream test parity.
GraphFrames GraphX fork versus Spark GraphX
The GraphFrames fork is a renamed snapshot of GraphX, not a separate graph
execution design: it duplicates 42 production files under
`org.apache.spark.graphframes.graphx`. Its substantive divergences are:
- Label propagation uses vector-based messages and deterministic
tie-breaking.
- Several implicit GraphX caches are removed, and Pregel message
checkpointing behavior differs.
- Shortest paths adds directed/undirected execution.
- The fork currently lacks Spark's July 2026 SVD++ combiner correction.
This PR deliberately uses canonical `org.apache.spark.graphx` instead of
importing a second GraphX implementation. It preserves undirected shortest
paths in the GraphFrames wrapper, does not carry the fork's
label-propagation/cache changes, and benefits from Spark's newer SVD++ fix. Any
generally useful GraphX changes should be proposed separately against Spark
GraphX.
At the time of this check, the GraphFrames Scala and Python jobs were green.
The overall workflow was still running, with separate documentation and lint
failures in GraphFrames code; no fixes or reruns were made as part of this
audit.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
