The failing test is the only thing I couldn't figure out. test_power_iteration_clustering seems to give different results in connect vs classic

The failing test is the only thing I couldn't figure out. test_power_iteration_clustering seems to give different results in connect vs classic

I faced it already, let me take a look.

The failing test is the only thing I couldn't figure out. test_power_iteration_clustering seems to give different results in connect vs classic

@Kimahriman Can you please take changes from #610? There are only few lines of code... It just will be faster, cause we are still having some governance issues and merging #610 may be slow. I think the problem is that PySpark Connect in 4.x does not preserve the order of rows after collect, explicit comparison of dicts id -> cluster should work, at least I hope so.

Thanks in advance!

The failing test is the only thing I couldn't figure out. test_power_iteration_clustering seems to give different results in connect vs classic

@Kimahriman Can you please take changes from #610? There are only few lines of code... It just will be faster, cause we are still having some governance issues and merging #610 may be slow. I think the problem is that PySpark Connect in 4.x does not preserve the order of rows after collect, explicit comparison of dicts id -> cluster should work, at least I hope so.

Thanks in advance!

Thought about that too but unfortunately doesn't appear to be the case. With that update:

poetry run pytest tests/test_graphframes.py::test_power_iteration_clustering -vv                         
========================================================================== test session starts ==========================================================================
platform darwin -- Python 3.11.13, pytest-8.3.5, pluggy-1.5.0 -- 
tests/test_graphframes.py::test_power_iteration_clustering PASSED                                                                                                 [100%]

=========================================================================== 1 passed in 8.81s ===========================================================================

SPARK_CONNECT_MODE_ENABLED=1 poetry run pytest tests/test_graphframes.py::test_power_iteration_clustering -vv
========================================================================== test session starts ==========================================================================
platform darwin -- Python 3.11.13, pytest-8.3.5, pluggy-1.5.0 -- 
tests/test_graphframes.py::test_power_iteration_clustering FAILED                                                                                                 [100%]

=============================================================================== FAILURES ================================================================================
____________________________________________________________________ test_power_iteration_clustering ____________________________________________________________________

spark = <pyspark.sql.connect.session.SparkSession object at 0x11e775110>

    def test_power_iteration_clustering(spark):
        vertices = [
            (1, 0, 0.5),
            (2, 0, 0.5),
            (2, 1, 0.7),
            (3, 0, 0.5),
            (3, 1, 0.7),
            (3, 2, 0.9),
            (4, 0, 0.5),
            (4, 1, 0.7),
            (4, 2, 0.9),
            (4, 3, 1.1),
            (5, 0, 0.5),
            (5, 1, 0.7),
            (5, 2, 0.9),
            (5, 3, 1.1),
            (5, 4, 1.3),
        ]
        edges = [(0,), (1,), (2,), (3,), (4,), (5,)]
        g = GraphFrame(
            v=spark.createDataFrame(edges).toDF("id"),
            e=spark.createDataFrame(vertices).toDF("src", "dst", "weight"),
        )
    
        clusters = {
            r["id"]: r["cluster"]
            for r in g.powerIterationClustering(k=2, maxIter=40, weightCol="weight")
            .sort("id")
            .collect()
        }
    
>       assert clusters == {0: 0, 1: 0, 2: 0, 3: 0, 4: 1, 5: 0}
E       assert {0: 0, 1: 0, 2: 0, 3: 0, 4: 0, 5: 1} == {0: 0, 1: 0, 2: 0, 3: 0, 4: 1, 5: 0}
E         
E         Common items:
E         {0: 0, 1: 0, 2: 0, 3: 0}
E         Differing items:
E         {4: 0} != {4: 1}
E         {5: 1} != {5: 0}
E         
E         Full diff:
E           {
E               0: 0,
E               1: 0,
E               2: 0,
E               3: 0,
E         -     4: 1,
E         ?        ^
E         +     4: 0,
E         ?        ^
E         -     5: 0,
E         ?        ^
E         +     5: 1,
E         ?        ^
E           }

tests/test_graphframes.py:161: AssertionError

Figured it out, it was a difference in the number of partitions of the initial edge DataFrame, must be something non-deterministic about the power iteration clustering?

Figured it out, it was a difference in the number of partitions of the initial edge DataFrame, must be something non-deterministic about the power iteration clustering?

Wow! Nice work! Yes, most probably it is not deterministic. Actually it is just a wrapper on top of SparkML

I also tried to fix some issues with the shading/assembly, I noticed with testing publishM2, the published jars still had scala and slf4j classes included. The one oddity is that the classes from the root project are included in the connect jar as well, not sure if that is intentional or not but couldn't figure out how to exclude them. Wonder if using the assembly plugin is overkill to try to shade a single dependency, and maybe the shading plugin would be simpler to use?

I also tried to fix some issues with the shading/assembly, I noticed with testing publishM2, the published jars still had scala and slf4j classes included. The one oddity is that the classes from the root project are included in the connect jar as well, not sure if that is intentional or not but couldn't figure out how to exclude them. Wonder if using the assembly plugin is overkill to try to shade a single dependency, and maybe the shading plugin would be simpler to use?

Putting classes from the root to connect is intentional to allow users to specify a single dependency and to have a ready to use connect-plugin instead of manually adding both connect and root. To be honest I'm not sure what would be the best to do it, I was trying to avoid user-facing "ClassNotFoundExpceptions" that are tricky to understand and resolve..

I will left this until the end of Sunday, @rjurney @SauronShepherd please, notify me if you need more time to review.

Putting classes from the root to connect is intentional to allow users to specify a single dependency and to have a ready to use connect-plugin instead of manually adding both connect and root. To be honest I'm not sure what would be the best to do it, I was trying to avoid user-facing "ClassNotFoundExpceptions" that are tricky to understand and resolve..

Based on the publishM2, graphframes is still a compile dependency to graphframes-connect, so if you used --packages graphframes:graphframes-connect you would end up with two copies of the same graphframes classes on your classpath. just a little awkward

Putting classes from the root to connect is intentional to allow users to specify a single dependency and to have a ready to use connect-plugin instead of manually adding both connect and root. To be honest I'm not sure what would be the best to do it, I was trying to avoid user-facing "ClassNotFoundExpceptions" that are tricky to understand and resolve..

Based on the publishM2, graphframes is still a compile dependency to graphframes-connect, so if you used --packages graphframes:graphframes-connect you would end up with two copies of the same graphframes classes on your classpath. just a little awkward

And that is another big problem of GraphFrames: I came to the project with a hope to get some scala experience, I was 100% not ready to work on the build-publish system just because I do not have enough experience for it. But someone had to do it, otherwise the all the new contributors lost the motivation soon if we wouldn't be able to make releases... Anyway thanks a lot, I will take a look!

It would be very cool also if you share how did you find it. I tried to run publishLocal and after that I unzip the jar and checked, it seems to me there was no scala and slf4j inside...

It would be very cool also if you share how did you find it. I tried to run publishLocal and after that I unzip the jar and checked, it seems to me there was no scala and slf4j inside...

Yeah that's basically what I did. They only appeared on the published connect JAR because of Compile / packageBin := assembly.value, which I guess makes the assembly the default package/publish target. And I usually just jar -tvf /path/to/jar to list things out without having to unzip it.