Skip to content

Pie chart visualization for mapper graphs - #105

Open
ishikaghosh2201 wants to merge 11 commits into
masterfrom
mapper_node_pie
Open

ishikaghosh2201 wants to merge 11 commits into
masterfrom
mapper_node_pie

Conversation

@ishikaghosh2201

Copy link
Copy Markdown
Collaborator

Description

Adds pie-chart visualizations to mapper graph nodes via a new draw_pie method, showing the categorical breakdown of original data points assigned to each node. Also adds optional point-count-based sizing (size_by_points), so nodes with more points render larger pie glyphs than nodes with fewer.

Motivation and Context

Mapper graph nodes represent clusters of original data points, but there was previously no way to visualise categorical metadata (independent of whatever built the graph) at the node level. This closes #90.

How has this been tested?

  • Added tests/test_pie_plot.py covering:
    • node_label_counts correctness on a small known point-to-node mapping
    • node_label_counts raising ValueError on manually-built graphs (no node_points attribute)
    • pie_plot/draw_pie smoke tests
    • size_by_points=True correctly scales zoom in proportion to each node's point count (verified via node-to-zoom correlation, not just min/max)
    • size_by_points=False preserves the original fixed-zoom behaviour
    • Equal-point-count edge case does not raise ZeroDivisionError
  • Manually verified in compute_mapper.ipynb against circles-dataset examples with both structured (angular quadrant) and random multi-category labels

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have incremented the version number in the pyproject.toml file if a new version needs to be pushed to pypi. Note that if the number isn't incremented, the package will not be pushed to pypi, which is useful if this PR is only for updating documentation.
  • My code follows the code style of this project and I have run make format to clean up the code with black.
  • My change requires a change to the documentation. I have updated the documentation as necessary and compiled locally to ensure it is clean.
  • I have added tests to cover my changes, and all new and existing tests passed (run make tests).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The updated dist_fit bounding logic can convert the integer search bound to a float (breaking the binary search), and a few plotting API/formatting issues should be corrected for correctness and consistency.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR adds pie-chart node glyph rendering for mapper graphs, enabling per-node visualization of categorical label composition, and introduces optional node sizing proportional to point counts (via node_points recorded during computeMapper).

Changes:

  • Add node_points tracking to computeMapper output graphs to support downstream per-node label aggregation.
  • Add node_label_counts, pie-glyph rendering utilities, and pie_plot, plus a MapperGraph.draw_pie() convenience method.
  • Add tests covering label counting, error conditions, smoke rendering, and size_by_points zoom scaling behavior; bump package version.
File summaries
File Description
cereeberus/cereeberus/compute/computemapper.py Records per-node original point indices (node_points) on mapper graph output.
cereeberus/cereeberus/draw/draw.py Adds pie plotting utilities (node_label_counts, _pie_image, pie_plot) and refactors edge drawing into _draw_edges.
cereeberus/cereeberus/reeb/mapper.py Adds MapperGraph.draw_pie() wrapper for pie-chart visualization.
tests/test_pie_plot.py New tests for pie plotting, label counting, and size scaling behavior.
tests/test_computemapper.py Adds regression test ensuring computeMapper populates node_points.
cereeberus/cereeberus/distance/interleave.py Adjusts dist_fit binary search bounding logic (note: unrelated to pie plotting).
pyproject.toml Version bump to 0.1.17 and minor formatting cleanup.
Review details

Suppressed comments (1)

cereeberus/cereeberus/draw/draw.py:316

  • The pie_plot signature isn’t formatted like the rest of this module (no spaces after commas, very long line). Reformatting (or running black) improves readability and avoids formatter churn in future diffs.
def pie_plot(R,labels,categories=None,colors=None,zoom=0.15,size_by_points=False,min_zoom=0.08,max_zoom=0.3,with_edges=True,with_legend=True,with_labels=True,cpx=0.1,cpy=0.1,ax=None):
  • Files reviewed: 7/8 changed files
  • Comments generated: 7
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

except ValueError: # infeasible assignment
low = mid + 1

high = min(high, best_bound - 1) # to tighten the upper bound on the search space. this tries to go higher
Comment thread cereeberus/cereeberus/draw/draw.py
Comment thread cereeberus/cereeberus/draw/draw.py
Comment thread cereeberus/cereeberus/reeb/mapper.py Outdated
Comment thread cereeberus/cereeberus/draw/draw.py
Comment thread cereeberus/cereeberus/reeb/mapper.py Outdated
Comment thread tests/test_pie_plot.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved critical and moderate issues affect edge rendering, sizing, autoscaling, transparency, and empty-graph handling.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

cereeberus/cereeberus/draw/draw.py:415

  • AnnotationBbox artists and networkx text labels do not contribute their positions to Axes.relim(). Therefore, when with_edges=False or the graph has no edges, autoscaling uses no node data and pies can be clipped or entirely outside the default limits; update the data limits from R.pos_f before calling autoscale_view().
    ax.relim()
    ax.autoscale_view()

cereeberus/cereeberus/draw/draw.py:296

  • fig.canvas.buffer_rgba() captures the opaque figure and axes backgrounds here, so every OffsetImage is a white square rather than a transparent circular pie. Those squares obscure edges, labels, and neighboring glyphs; make the figure/axes patches transparent (and hide the axes) before rendering the image.
    fig = plt.figure(figsize=(px / dpi, px / dpi), dpi=dpi)
    pie_ax = fig.add_axes([0, 0, 1, 1])
    pie_ax.set_aspect("equal")

cereeberus/cereeberus/reeb/mapper.py:143

  • The method defaults to cpx=cpy=1.0, while pie_plot and ReebGraph.draw use 0.1; consequently MapperGraph.draw_pie() draws multi-edges with a much larger, inconsistent curvature than the helper it delegates to. Align these defaults with pie_plot unless the larger curvature is intentional and documented.
    def draw_pie(self, labels, categories=None, colors=None, zoom=0.15, size_by_points=False, min_zoom=0.08, max_zoom=0.3, with_edges=True, with_legend=True, with_labels=True, cpx=1.0, cpy=1.0, ax=None):
  • Files reviewed: 6/7 changed files
  • Comments generated: 3
  • Review effort level: Lite

Comment thread cereeberus/cereeberus/draw/draw.py
Comment thread cereeberus/cereeberus/draw/draw.py Outdated
Comment thread cereeberus/cereeberus/draw/draw.py Outdated
Co-authored-by: ishikaghosh2201 <112980412+ishikaghosh2201@users.noreply.github.com>
ishikaghosh2201 and others added 3 commits September 9, 2026 15:13
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved moderate issues affect pie sizing, visibility, transparency, multiedge handling, and wrapper defaults.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (4)

cereeberus/cereeberus/draw/draw.py:413

  • AnnotationBbox objects are added with ax.add_artist, so they do not contribute their data coordinates to the axes' data limits. Consequently relim()/autoscale_view() only sees edge lines (or no data at all when with_edges=False), and disconnected nodes or edge-less graphs can place the pie glyphs outside the visible plot. Include the node positions in the data limits before autoscaling.
    ax.relim()
    ax.autoscale_view()

cereeberus/cereeberus/draw/draw.py:296

  • buffer_rgba() captures the figure and axes patches, which are opaque white by default, so every OffsetImage is a square white tile rather than a transparent pie glyph. This is visible on non-white axes and when glyphs overlap; make both patches transparent before rendering.
    fig = plt.figure(figsize=(px / dpi, px / dpi), dpi=dpi)
    pie_ax = fig.add_axes([0, 0, 1, 1])
    pie_ax.set_aspect("equal")

cereeberus/cereeberus/draw/draw.py:37

  • Using keys=True fixes the tuple length, but line_loop_index still assumes key 0 precedes key 1 and that key 1 proves a parallel edge. NetworkX keys are identifiers whose values and iteration order can change; for example, removing key 0 and re-adding it can yield [1, 0], making line_index.remove(...) raise ValueError. Classify duplicate (u, v) pairs instead of relying on numeric key values.
    edge_list = list(R.edges(keys=True))

cereeberus/cereeberus/reeb/mapper.py:143

  • The wrapper forwards cpx and cpy as 1.0 by default, while pie_plot and the existing ReebGraph.draw use 0.1. A default draw_pie() therefore changes the established edge curvature for multiedges instead of using the delegated plotting function's defaults; keep these wrapper defaults aligned.
    def draw_pie(self, labels, categories=None, colors=None, zoom=0.15, size_by_points=False, min_zoom=0.08, max_zoom=0.3, with_edges=True, with_legend=True, with_labels=True, cpx=1.0, cpy=1.0, ax=None):
  • Files reviewed: 6/7 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread cereeberus/cereeberus/draw/draw.py Outdated
ishikaghosh2201 and others added 2 commits September 9, 2026 15:27
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Two moderate issues in drawing and edge handling remain unresolved.

Review details

Suppressed comments (2)

cereeberus/cereeberus/draw/draw.py:419

  • AnnotationBbox and text artists do not contribute their positions to Axes.relim(), so these calls only autoscale to edge line data. Isolated mapper nodes (or any call with with_edges=False) can therefore fall outside the limits and have their pies clipped or invisible. Add every R.pos_f coordinate to the axis data limits before calling autoscale_view.
    ax.relim()
    ax.autoscale_view()

cereeberus/cereeberus/draw/draw.py:37

  • Although keys=True fixes the tuple-length failure, line_loop_index still assumes that parallel edges are keyed exactly 0 and 1, with key 0 encountered first. A valid MultiDiGraph with only key 1 (for example after removing key 0) reaches the key-1 branch and .index(..., 0) raises ValueError; determine parallelism from endpoint groups/counts instead of key values.
    edge_list = list(R.edges(keys=True))
  • Files reviewed: 6/7 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

ishikaghosh2201 and others added 2 commits September 9, 2026 15:38
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add pychart visualization

3 participants