docs: sync connected components docs with the code - #903
Merged
Merged
Conversation
The configuration page and the connected components argument list had drifted from the implementation. 13-configurations.md: - the algorithm table listed only "graphframes" and "graphx" and named "graphframes" as the default. The default is "two_phase" (ConnectedComponents.scala), and "randomized_contraction" is also supported; "graphframes" is a deprecated alias that logs a warning; - useLabelsAsComponents was documented as defaulting to true; it defaults to false (mixins.scala); - broadcastthreshold and checkpointinterval were documented as applying only when the algorithm is "graphframes". broadcastthreshold applies to "two_phase", checkpointinterval to "two_phase" and "randomized_contraction"; - broadcastthreshold = -1, which hands skew handling to AQE, was not documented at all; - every example set the deprecated "graphframes" value; - the local checkpoints note claimed they are less error prone. They are less reliable: they do not survive the loss of an executor; - checkpointing did not mention the spark.checkpoint.dir fallback used when SparkContext.setCheckpointDir is not reachable, nor that "two_phase" in AQE mode uses Spark's own DataFrame checkpointing. 05-traversals.md: - the connected components argument list documented checkpoint_interval, broadcast_threshold and use_labels_as_components, which match neither the Python nor the Scala API. Renamed to the real names, using the "(Python) / (Scala)" convention already used elsewhere on the page; - the maxIter default is described without naming a constant, since the Python and Scala defaults differ by one; - one typo in the shortest paths argument list: is_direted -> is_directed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
|
Thank you for the contribution! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
The configuration page and the connected components argument list had drifted from the
implementation. Every correction below is against code on
main.docs/src/04-user-guide/13-configurations.mdgraphframes,graphx; defaultgraphframestwo_phase(ConnectedComponents.scala:56), andgraphframesis a deprecated alias that logs a warning (:86-88)useLabelsAsComponentsdefaults totruefalse(mixins.scala:154)broadcastthreshold"only used when the algorithm isgraphframes"two_phase(ConnectedComponents.scala:146-163)checkpointinterval"only used when the algorithm isgraphframes"two_phaseandrandomized_contraction(:149,:158,:170)broadcastthreshold = -1(AQE mode)mixins.scala:174-178)Also: all six code examples set the deprecated
"graphframes"value, so following the pageproduced a deprecation warning. They now use
"two_phase".Two things the page never mentioned, both of which matter in practice:
spark.checkpoint.dirconfiguration whenSparkContext.setCheckpointDiris not set (TwoPhase.scala:278,RandomizedContraction.scala:69). This is the only route available whensparkContextisnot reachable, for example from a Spark Connect client.
two_phasein AQE mode (broadcastthreshold = -1) uses Spark's own DataFrame checkpointingrather than its own named sub-directory (
TwoPhase.scala:534), unlike the other paths.docs/src/04-user-guide/05-traversals.mdThe connected components argument list documented
checkpoint_interval,broadcast_thresholdanduse_labels_as_components. Those names match neither API — thePython parameters are
checkpointInterval,broadcastThresholdanduseLabelsAsComponents,and the Scala setters are
setCheckpointInterval,setBroadcastThresholdandsetUseLabelsAsComponents. Copying from the page raisedTypeError. They now use the(Python) / (Scala)convention already used further up the same page forall_paths.maxIteris no longer described as defaulting toInteger.MAX_VALUE, since the Python andScala defaults differ by one; it is described as effectively unlimited instead.
One typo outside the connected components section is also fixed: the shortest paths argument
list documented
is_direted; the parameter isis_directed. Happy to split that out if youwould rather keep this PR strictly scoped.
Note that
checkpoint_intervalis deliberately left untouched in the shortest paths, cyclesdetection and aggregate neighbors sections — it is the correct Python name for those methods.
Only
connectedComponentsuses camelCase.Why are the changes needed?
This is the page a user lands on when tuning connected components for a large job. As written
it gave the wrong default component ID type, hid the algorithm the project itself recommends
as roughly 5x faster, documented three parameter names that cannot be typed, and stated the
reliability of local checkpoints backwards.
Verification
Documentation only; no code, build or CI files are touched.
sbt docs/laikaAST)mainbaselinecodespell .(2.4.2, as in the spellcheck workflow)GraphFramesConf.scala:36,38ConnectedComponents.run()sbt docs/laikaHTMLcould not be run to completion, for a reason unrelated to this PR:docs/generateAtomFeedfails on a clean checkout withDateTimeParseException: Text '1970-01-01:T00:00:00Z' could not be parsed at index 10(
LaikaCustoms.scala:67and:87hardcode that literal, which carries a stray colon).