mirror of
https://github.com/safishamsi/graphify.git
synced 2026-07-22 15:21:34 +00:00
fix(export): coerce non-scalar attrs in graphml; harden pipe-close flush
#1831 — `graphify export graphml` crashed on any dict/list-valued attribute (per-node metadata dict, graph-level hyperedges list) because nx.write_graphml only accepts scalars; a real ~2,300-node graph failed every export and left a 0-byte .graphml behind. to_graphml now coerces None->"" and JSON-serializes non-scalars across graph/node/edge scopes (int/float/bool/str pass through), and writes atomically via a temp file so a failed export can't leave a partial file. Closes #1830. #1807 followup — adopt @varuntej07's explicit in-guard sys.stdout.flush() from #1811: piped stdout is block-buffered, so a small fully-buffered output would only flush at interpreter shutdown (outside the guard), where a closed-pipe reader escapes as a noisy shutdown error and nonzero exit. Flushing inside the try closes that gap. Closes #1811. Reported by @hofmockel (#1831) and @varuntej07 (#1807/#1811). Co-Authored-By: hofmockel <hofmockel@users.noreply.github.com> Co-Authored-By: varuntej07 <varuntej07@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
hofmockel
varuntej07
Claude Opus 4.8
parent
5f57c46b92
commit
a46eee49ef
+3
-1
@@ -6,7 +6,9 @@ Full release notes with details on each version: [GitHub Releases](https://githu
|
||||
|
||||
- Fix: Visual Studio *solution folder* nodes no longer embed the absolute scan path (including the local username) in their `id` and `source_file` (#1789, thanks @fremat79). A solution folder is a virtual grouping, not a file — VS writes its name as both the display name and the "path" — but `extract_sln` resolved it to an absolute filesystem path anyway and keyed the node id off that. The CLI's id-relativization pass only remaps ids of real files in the scan set, so a virtual folder never matched and its absolute id survived into a committed `graph.json` (e.g. `id=/Users/<name>/proj/Plugins` instead of `id=plugins`). Solution folders are now detected (name == path) and keyed off the folder name only; real project files still resolve as before. (The earlier fix covered `.csproj`/`.sln` file nodes but missed the virtual folders — this completes it.)
|
||||
|
||||
- Fix: the CLI no longer crashes with exit code 255 when a downstream reader closes the pipe early (#1807, thanks @varuntej07). Truncating output with `head`, PowerShell's `Select-Object -First N`, or `sed q` disconnected the reader mid-write, graphify hit an unhandled `BrokenPipeError` (or `OSError(EINVAL)` on Windows) and exited 255 — so CI wrappers and agent harnesses that both trim output and check the exit code read a successful query as a command failure. An early-closing reader is now treated as success: stdout is redirected to devnull so the interpreter's shutdown flush can't raise again, and the process exits 0.
|
||||
- Fix: the CLI no longer crashes with exit code 255 when a downstream reader closes the pipe early (#1807 / #1811, thanks @varuntej07). Truncating output with `head`, PowerShell's `Select-Object -First N`, or `sed q` disconnected the reader mid-write, graphify hit an unhandled `BrokenPipeError` (or `OSError(EINVAL)` on Windows) and exited 255 — so CI wrappers and agent harnesses that both trim output and check the exit code read a successful query as a command failure. An early-closing reader is now treated as success: stdout is flushed inside the guard (piped stdout is block-buffered, so a small output would otherwise only flush at interpreter shutdown, where the error escapes as a noisy "Exception ignored" and a nonzero exit), then redirected to devnull so the shutdown flush can't raise again, and the process exits 0.
|
||||
|
||||
- Fix: `graphify export graphml` no longer crashes on dict- or list-valued attributes (#1831 / #1830, thanks @hofmockel). `nx.write_graphml` only accepts scalar values, so a per-node `metadata` dict or the graph-level `hyperedges` list raised `GraphML does not support type <class 'dict'/'list'>` and failed the entire export — on a real ~2,300-node graph, every attempt. `to_graphml` now coerces `None -> ""` and JSON-serializes non-scalars across graph/node/edge scopes (GraphML-native int/float/bool/str pass through unchanged), and writes atomically via a temp file so a failed export no longer leaves a 0-byte `.graphml` that downstream tooling mistakes for a completed one.
|
||||
|
||||
- Fix: `.nox/` (nox virtualenvs) is now skipped during detection alongside `.tox/` (#1804, thanks @igorregoir-lgtm). nox is tox's successor and creates a `.nox/` tree of the same shape, but only `.tox` was in the skip set — so a repo with a nox env got its site-packages fully indexed (one real repo came out 91% venv noise: 6,720 of 7,365 nodes from `.nox/`) and semantic extraction burned tokens reading venv docs.
|
||||
|
||||
|
||||
@@ -464,6 +464,11 @@ def main() -> None:
|
||||
agent harnesses read a successful query as a command failure (#1807)."""
|
||||
try:
|
||||
_run_cli()
|
||||
# Flush explicitly, inside the guard. Piped stdout is block-buffered, so a
|
||||
# small fully-buffered output would otherwise only flush at interpreter
|
||||
# shutdown — outside this try — where a reader that closed the pipe surfaces
|
||||
# as a noisy "Exception ignored on flushing sys.stdout" and a nonzero exit.
|
||||
sys.stdout.flush()
|
||||
except BrokenPipeError:
|
||||
_silence_broken_pipe()
|
||||
except OSError as exc:
|
||||
|
||||
+34
-6
@@ -911,16 +911,44 @@ def to_graphml(
|
||||
for _, _, attrs in H.edges(data=True):
|
||||
for k in [k for k in attrs if k.startswith("_")]:
|
||||
del attrs[k]
|
||||
# nx.write_graphml raises ValueError on None attribute values; replace with "".
|
||||
# nx.write_graphml only accepts scalar attribute values: None raises, and a
|
||||
# dict/list value (e.g. a per-node `metadata` dict, or the graph-level
|
||||
# `hyperedges` list set by attach_hyperedges()) raises
|
||||
# "GraphML does not support type <class 'dict'/'list'> as data values" (#1831).
|
||||
# Coerce None -> "" and non-scalars -> a JSON string, across all three scopes.
|
||||
def _graphml_safe(val):
|
||||
if val is None:
|
||||
return ""
|
||||
if isinstance(val, bool) or isinstance(val, (int, float, str)):
|
||||
return val # GraphML-native scalars pass through unchanged
|
||||
try:
|
||||
return json.dumps(val, default=str, sort_keys=True)
|
||||
except (TypeError, ValueError):
|
||||
return str(val)
|
||||
|
||||
for key, val in list(H.graph.items()):
|
||||
H.graph[key] = _graphml_safe(val)
|
||||
for node_id in H.nodes():
|
||||
for key, val in list(H.nodes[node_id].items()):
|
||||
if val is None:
|
||||
H.nodes[node_id][key] = ""
|
||||
H.nodes[node_id][key] = _graphml_safe(val)
|
||||
for u, v in H.edges():
|
||||
for key, val in list(H.edges[u, v].items()):
|
||||
if val is None:
|
||||
H.edges[u, v][key] = ""
|
||||
nx.write_graphml(H, output_path)
|
||||
H.edges[u, v][key] = _graphml_safe(val)
|
||||
|
||||
# Write atomically: a mid-serialization error otherwise leaves a 0-byte
|
||||
# .graphml on disk that downstream tooling mistakes for a completed export
|
||||
# (#1831). Write to a sibling temp file, then replace on success.
|
||||
out = Path(output_path)
|
||||
tmp = out.with_name(out.name + ".tmp")
|
||||
try:
|
||||
nx.write_graphml(H, str(tmp))
|
||||
os.replace(str(tmp), str(out))
|
||||
finally:
|
||||
if tmp.exists():
|
||||
try:
|
||||
tmp.unlink()
|
||||
except OSError:
|
||||
pass
|
||||
|
||||
|
||||
def to_svg(
|
||||
|
||||
@@ -29,3 +29,21 @@ def test_help_survives_reader_closing_pipe_early():
|
||||
rc = producer.wait()
|
||||
# 0 (our handled-and-succeed convention). Never the 255 unhandled-exception code.
|
||||
assert rc == 0, f"expected clean exit after early pipe close, got {rc}"
|
||||
|
||||
|
||||
def test_small_buffered_output_survives_reader_that_reads_nothing():
|
||||
"""A short, fully-buffered output (piped stdout is block-buffered) only flushes
|
||||
at exit. If the reader closed the pipe without reading, that flush must be
|
||||
handled inside the CLI's guard and exit 0, not escape as a shutdown error."""
|
||||
producer = subprocess.Popen(
|
||||
[PYTHON, "-m", "graphify", "--version"], stdout=subprocess.PIPE
|
||||
)
|
||||
reader = subprocess.Popen(
|
||||
[PYTHON, "-c", "pass"], # exits immediately, reads nothing
|
||||
stdin=producer.stdout,
|
||||
stdout=subprocess.DEVNULL,
|
||||
)
|
||||
producer.stdout.close()
|
||||
reader.wait()
|
||||
rc = producer.wait()
|
||||
assert rc == 0, f"expected clean exit when reader reads nothing, got {rc}"
|
||||
|
||||
@@ -99,6 +99,48 @@ def test_to_graphml_tolerates_none_attribute_values():
|
||||
content = out.read_text()
|
||||
assert "<graphml" in content
|
||||
|
||||
def test_to_graphml_tolerates_dict_and_list_attribute_values():
|
||||
"""nx.write_graphml only accepts scalars; a dict/list attribute (per-node
|
||||
metadata, or the graph-level hyperedges list) used to crash the whole export.
|
||||
to_graphml must JSON-serialize them across graph/node/edge scopes (#1831)."""
|
||||
import networkx as nx
|
||||
G = make_graph()
|
||||
communities = cluster(G)
|
||||
a_node = next(iter(G.nodes()))
|
||||
G.nodes[a_node]["metadata"] = {"kind": "file", "size": 12}
|
||||
G.nodes[a_node]["tags"] = ["x", "y"]
|
||||
if G.number_of_edges():
|
||||
u, v = next(iter(G.edges()))
|
||||
G.edges[u, v]["ctx"] = {"k": "v"}
|
||||
G.graph["hyperedges"] = [{"nodes": [a_node], "label": "h"}]
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
out = Path(tmp) / "graph.graphml"
|
||||
to_graphml(G, communities, str(out)) # must not raise
|
||||
H = nx.read_graphml(str(out))
|
||||
assert json.loads(H.nodes[a_node]["metadata"]) == {"kind": "file", "size": 12}
|
||||
assert json.loads(H.nodes[a_node]["tags"]) == ["x", "y"]
|
||||
assert json.loads(H.graph["hyperedges"]) == [{"nodes": [a_node], "label": "h"}]
|
||||
assert not (Path(tmp) / "graph.graphml.tmp").exists()
|
||||
|
||||
|
||||
def test_to_graphml_preserves_native_scalar_types():
|
||||
"""Coercion must leave GraphML-native scalars (int/float/bool/str) untouched,
|
||||
only stringifying non-scalars (#1831)."""
|
||||
import networkx as nx
|
||||
G = nx.Graph()
|
||||
G.add_node("a", count=3, ratio=0.5, flag=True, name="x")
|
||||
G.add_node("b")
|
||||
G.add_edge("a", "b")
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
out = Path(tmp) / "g.graphml"
|
||||
to_graphml(G, {0: ["a", "b"]}, str(out))
|
||||
H = nx.read_graphml(str(out))
|
||||
assert H.nodes["a"]["count"] == 3
|
||||
assert H.nodes["a"]["ratio"] == 0.5
|
||||
assert H.nodes["a"]["flag"] is True
|
||||
assert H.nodes["a"]["name"] == "x"
|
||||
|
||||
|
||||
def test_to_html_creates_file():
|
||||
G = make_graph()
|
||||
communities = cluster(G)
|
||||
|
||||
Reference in New Issue
Block a user