Skip to content

Conversation

@ericfe-google
Copy link
Contributor

@ericfe-google ericfe-google commented Feb 21, 2025

feat: Support visualization of graph queries by adding the --graph argument.

@ericfe-google ericfe-google requested review from a team as code owners February 21, 2025 18:37
@product-auto-label product-auto-label bot added the size: l Pull request size is large. label Feb 21, 2025
@product-auto-label product-auto-label bot added the api: bigquery Issues related to the googleapis/python-bigquery-magics API. label Feb 21, 2025
@ericfe-google ericfe-google changed the title Graph feat: support visualization of graph queries by adding the --graph argument. Feb 25, 2025
…raph server is already running, due to another graph query having run previously.
result = result.to_dataframe(**dataframe_kwargs)

if args.graph and _supports_graph_widget(result):
_add_graph_widget(result)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we do a warning if they asked for --graph but the results didn't support it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Long-term, I view --graph as a temporary flag to avoid breaking the standard code path while the graph implementation is not stable yet. Eventually, I would like to see the flag go away, and the graph visualization just be automatic in response to running a graph query.

So, I don't think such a warning is warranted right now.

self.handle_post_query()


atexit.register(GraphServer.stop_server)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with atexit, what does it do and why do we need it? Might be worth adding a few comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the code is pretty much self-explanatory. It's necessary to prevent unit tests from hanging, due to the server background thread still running at the end of all of the tests.

GraphServer._server = None


class GraphServerHandler(http.server.SimpleHTTPRequestHandler):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd love a few docstrings on these methods explaining their purpose. I can make some guesses based on the method names, but they are pretty generic names, so it's hard to interpret the intention.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some.

@tswast tswast added the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 10, 2025
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 10, 2025
from google.cloud.spanner_v1.types import StructType, Type, TypeCode
import networkx
from spanner_graphs.conversion import (
columns_to_native_numpy,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tswast tswast merged commit 3c054f5 into main Mar 11, 2025
25 checks passed
@tswast tswast deleted the graph branch March 11, 2025 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigquery Issues related to the googleapis/python-bigquery-magics API. size: xl Pull request size is extra large.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants