Switch Plottable to a Protocol by exrhizo · Pull Request #667 · graphistry/pygraphistry

exrhizo · 2025-06-18T22:46:33Z

TODO

Think about pattern for Kusto and Spanner
Put back in Kusto
Put back in Spanner
Add notes / readme to help future developers with typing
Changelog

Typecheck

Getting close to types passing, still have these mysterious cases, should fallback on the PlottableBase afiac

graphistry/plotter.py:17: error: Cannot determine type of "_edges" in base class "HeterographEmbedModuleMixin"  [misc]
graphistry/plotter.py:17: error: Cannot determine type of "_edges" in base class "DGLGraphMixin"  [misc]
graphistry/embed_utils.py:307: error: Cannot determine type of "_nodes"  [has-type]
graphistry/embed_utils.py:308: error: Cannot determine type of "_nodes"  [has-type]
graphistry/embed_utils.py:312: error: Cannot determine type of "_edges"  [has-type]
graphistry/embed_utils.py:313: error: Cannot determine type of "_edges"  [has-type]
graphistry/dgl_utils.py:258: error: Cannot determine type of "_edges"  [has-type]

lmeyerov · 2025-06-19T00:01:23Z

    ):
        return Plotter().from_igraph(ig, node_attributes, edge_attributes, load_nodes, load_edges)
-    from_igraph.__doc__ = Plotter.from_igraph.__doc__
+    from_igraph.__doc__ = Plotter.from_igraph.__doc__ # type: ignore


This doesn't seem to work, when I inspect in a notebook and it didn't work before my changes

I also notice that the docs don't show the PyGraphistry functions anyway, so it doesn't look like adding docs here is meaningful

exrhizo

Okay put back in spanner and kusto, fixed issues and responded to comments

lmeyerov · 2025-06-19T18:00:20Z

            logger.info('-Reusing Existing Node Featurization')
-            return res._node_features, res._node_target, res
+            # NOTE: Wasn't sure if " res._node_target is not None" ought to have been a condition for this code path
+            node_target = res._node_target if res._node_target is not None else pd.DataFrame()


i missed it on the first pass of these, but i wonder if for these df.empty y stubs, we need len(y.index) == len(X.index), even if no columns... i can imagine downstream code being confused on shape mismatch

as long as CI passes, don't need here, but more of a note for future PRs to think about

lmeyerov · 2025-06-19T18:03:32Z

        load_nodes = True, load_edges = True
    ):
        return Plotter().from_igraph(ig, node_attributes, edge_attributes, load_nodes, load_edges)
-    from_igraph.__doc__ = Plotter.from_igraph.__doc__


don't we want to copy the doc str in cases like these? like we inherit (maybe) when it is x = x, but not when def x(): x()

(unsure where else)

I thought I replied somewhere, that this didn't work before and it isn't working now, I don't know why not

is in, from_igraph.__doc__ maybe because it is static doesn't get set

actually the module level exports got have doc pages afaik

maybe it works for help(x) but sphinx doesn't see it

I'll look at this more

lmeyerov

approved - see comments, a few things worth noting:

seems worth trying on rtd.org , see slack for getting you a maintainer acct
we still likely need to copy docstr when doing def x(): return x(), as that's a new method, vs a pointer
i saw some type ignores on transform unsure if more
CHANGELOG.md

would you like to try publishing, or should i?

Hide the inherttited members for mixins

fix lint fix lint

exrhizo requested a review from lmeyerov June 18, 2025 22:46

exrhizo added the WIP label Jun 18, 2025