Skip to content

Conversation

james-willis
Copy link
Collaborator

This change lets useLabelsAsComponents to be set locally rather than globally via a spark config. This lets libraries that consume graphframes to avoid leaky abstraction and enables other usecases where affecting the global state of the cluster is inappropriate. See discussion here: apache/sedona#2098

What changes were proposed in this pull request?

  • Adding the WithUseLabelsAsComponents mixing and add to ConnectedComponents
  • change default to false to avoid breaking changes to the ConnectedComponents interface

Why are the changes needed?

  1. avoid making a breaking change to the return type of this function
  2. enable cleaner configuration of the algorithm when it is an implementation detail of a downstream interface

@james-willis james-willis force-pushed the useLabelsAsComponents branch 2 times, most recently from 08e1c2f to 54c9d98 Compare July 15, 2025 06:03
@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.74%. Comparing base (06ee372) to head (54c9d98).
Report is 12 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

❗ There is a different number of reports uploaded between BASE (06ee372) and HEAD (54c9d98). Click for more details.

HEAD has 3 uploads less than BASE
Flag BASE (06ee372) HEAD (54c9d98)
4 1
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #632      +/-   ##
==========================================
- Coverage   87.82%   80.74%   -7.08%     
==========================================
  Files          22       24       +2     
  Lines        1092     1184      +92     
  Branches      124      148      +24     
==========================================
- Hits          959      956       -3     
- Misses        133      228      +95     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@james-willis james-willis force-pushed the useLabelsAsComponents branch 3 times, most recently from a1421b4 to 928c6a6 Compare July 15, 2025 07:06
@SemyonSinchenko
Copy link
Collaborator

SemyonSinchenko commented Jul 15, 2025

@james-willis I think it is my fault, I was going to work on the docs about how to contribute to the spark-connect plugin but as always I failed due to limited time...

tldr:

  1. U need to update the protobuf message here: https://github.com/graphframes/graphframes/blob/master/graphframes-connect/src/main/protobuf/graphframes.proto#L62
  2. U need to update the plugin util here: https://github.com/graphframes/graphframes/blob/master/graphframes-connect/src/main/scala/org/apache/spark/sql/graphframes/GraphFramesConnectUtils.scala#L99
  3. U need to re-generate python files from protobuf:

@SemyonSinchenko
Copy link
Collaborator

Or just add it to the core and I will do all the corresponding updates of PySpark APIs (both classic and connect)

@james-willis james-willis force-pushed the useLabelsAsComponents branch from 928c6a6 to 42af82e Compare July 15, 2025 07:23
@james-willis
Copy link
Collaborator Author

Surely this time is the charm!

my local python tests aren't set up so I've been using the CI as a test platform.

@james-willis james-willis force-pushed the useLabelsAsComponents branch from 42af82e to 01de941 Compare July 15, 2025 07:50
@james-willis james-willis marked this pull request as ready for review July 15, 2025 08:53
@james-willis james-willis changed the title [WIP] allow useLabelsAsComponents to be set locally on the ConnectedComponents instance allow useLabelsAsComponents to be set locally on the ConnectedComponents instance Jul 15, 2025
@SemyonSinchenko SemyonSinchenko merged commit f3b3b20 into graphframes:master Jul 15, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants