Fix viz bugs and bring graphblas/viz.py under test and coverage - #597
Open
eriknw wants to merge 5 commits into
Open
Fix viz bugs and bring graphblas/viz.py under test and coverage#597eriknw wants to merge 5 commits into
eriknw wants to merge 5 commits into
Conversation
eriknw
marked this pull request as ready for review
August 4, 2026 16:07
eriknw
force-pushed
the
17-viz-fixes
branch
2 times, most recently
from
August 5, 2026 00:06
491bbf3 to
cf2817d
Compare
viz.py previously had zero tests. Nine Agg-backend smoke tests cover spy (default, centered, explicit axes), draw (renders nodes/labels, rejects non-Matrix), and datashade (single, list, grid, empty agg). Every optional dependency is guarded with importorskip so a minimal-environment run sees clean skips: without matplotlib the whole module skips; without datashader the four datashade tests skip and the rest run (both verified by simulation).
spy() bound the ``fig`` local only when it created the figure itself, so ``spy(A, figure=fig)`` crashed creating the axes and ``spy(A, axes=ax)`` crashed in the auto-markersize path (found while writing the smoke tests). The figure kwarg is now used when given, and auto-markersize reads dpi from ``axes.figure``, which is correct in all three call forms. Regression tests cover both previously-crashing forms.
…iz.draw Two bugs reported in the pyOpenSci review. Both change viz.py, so they are itemized in a single commit. gh-473: datashade binned element (r, c) into the pixel spanning [c, c+1) x [r, r+1), so it rendered centered at (c+0.5, r+0.5), half a cell off the tick labeled (c, r); spy centers the same element exactly on the tick. The axis limits now use the imshow integer-center convention (-0.5 to n-0.5), making datashade agree with spy. The issue's other symptom (elements invisible until zoom-out) does not reproduce on the current holoviews/hvplot/bokeh stack; it was a 2023-era library artifact. gh-474: reciprocal directed edges drew as coincident straight lines with both weight labels on the same midpoint, hiding one weight. Reciprocal pairs now draw with an arc (connectionstyle arc3, rad 0.1) and matching label placement, so both arrows and both weights are visible; other edges and self-loops stay straight. The gh-474 fix needs networkx 3.3, the release that gave draw_networkx_edge_labels its connectionstyle parameter. We support networkx >=2.8, so draw() feature-detects that parameter and keeps the previous straight rendering when it is missing; passing it to an older networkx raises TypeError. Curving the edges but not the labels would be worse than not curving at all, since the labels would sit back on the shared chord midpoint (the overlap gh-474 is about) and would no longer track their arrows. Both fixes carry display-free regression tests: datashade pixel centers match spy's convention, and draw places the two weight labels at separated anchors. The label check measures separation as a fraction of the edge length rather than comparing positions for exact inequality, because networkx returns two midpoint anchors that differ by floating-point noise even when the labels coincide on screen. A third test stands in a pre-3.3 signature to cover the fallback.
Removes the coverage omit block (viz.py was its only entry, with a TODO to un-omit once tests existed; they do now). The viz test module covers 87.6% of viz.py counting statements and branches, 91.1% counting statements alone. The misses are the optional-import failure path, the interactive plt.show() path, and guards for inputs the tests do not construct: matrices too large for int64 indices, ragged aggregator grids, and caller-supplied opts_kwargs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
graphblas/viz.py had zero tests and was omitted from coverage. This branch
adds smoke tests, fixes the bugs the tests uncovered plus two reported in the
pyOpenSci review, and un-omits the module.
and datashade, with importorskip guards so minimal environments see clean
skips.
spy(A, figure=fig)andspy(A, axes=ax)both crashed because thefiglocal was only bound whenspy created the figure itself. Regression tests cover both forms.
([pyos][visualization] Reciprocal directed edges overlap #474) (41045ad): datashade rendered each element half a cell off
from the tick, disagreeing with spy; it now uses the imshow
integer-center convention. Reciprocal directed edges drew as coincident
lines with one weight label hidden; they now draw as arcs with separated
labels. The arc rendering needs networkx 3.3, so draw() feature-detects
the connectionstyle parameter and keeps the straight rendering on older
networkx.
was its only entry); 87.6% covered counting statements and branches.
Stack created with GitHub Stacks CLI • Give Feedback 💬