Open
Conversation
Add embedded pie chart web component, templates, static assets, and pie_chart_no_cache setting. Store raw JSON on graph for pie chart data access.
7b29c2d to
5251937
Compare
Owner
|
I haven't looked at the code, but the interface looks great, only a few small things remain:
|
Co-Authored-By: Claude Opus 4.6
The pie chart was the only visualization reading graph._raw_JSON directly, bypassing config filters like excludeFinalWinnerAndEliminatedCandidate. New converter in visualizer/pie/graphToRCtab.py builds RCTab-compatible JSON from the same processed Graph that bar chart and sankey use. Co-Authored-By: Claude Opus 4.6
- Pass textForWinner and excludeFinalWinnerAndEliminatedCandidate - Set currentRound to numRounds so pie loads on last round - Rebuilt standalone component with new props Co-Authored-By: Claude Opus 4.6
Following the hideSankey/hideTabular pattern: model field, config JS variable, settings checkbox, and tab hiding in hideTabsBasedOnConfig(). Co-Authored-By: Claude Opus 4.6
Covers structure validation, tally correctness, eliminations, multi-winner surplus, inactive ballots, no-threshold elections, excludeFinalWinner flag, and the _stringify helper. Co-Authored-By: Claude Opus 4.6
Co-Authored-By: Claude Opus 4.6
Use a coprime stride near N/2 to reorder the color palette so that visually similar colors are spread apart around the pie chart. Co-Authored-By: Claude Opus 4.6
Replace color scramble with tally interleaving: alternate largest and smallest candidates so small slices always have a large neighbor, reducing label overlap. Inactive Ballots always last. Fix color generation to use actual tally entry count instead of config.numCandidates, which excluded Inactive Ballots and caused color wrapping on large elections. Co-Authored-By: Claude Opus 4.6
Co-Authored-By: Claude Opus 4.6
Per-round reordering broke D3 pie animation because candidate positions shifted between rounds. Now the interleave order is stable across all rounds, just dropping eliminated candidates. Co-Authored-By: Claude Opus 4.6
Tests pie vistype rendering, embedly redirect, hidePie config toggle, and cloudflare cache purge including pie URLs. Tests graphToRCtab with candidate names containing HTML, quotes, and other special characters. Co-Authored-By: Claude Opus 4.6
Author
|
Added 7 new tests for the pie chart feature: testSimple.py (3 new + 1 updated):
testGraphToRCtab.py (4 new in
|
Co-Authored-By: Claude Opus 4.6
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.
Summary
Response to feedback
Done.
Done. The pie chart now reads from the processed Graph (via a new
graphToRCtabconverter) instead of raw JSON, so it respects all config filters includingexcludeFinalWinnerAndEliminatedCandidate.We haven't been doing anything special about this. Not sure what the issue is.
Done.
textForWinnerandexcludeFinalWinnerAndEliminatedCandidateare now passed through from the config.We do this by default. Do you need it to work from active votes if you don't have it this way?
Done.
Done. Added
hidePiemodel field, config JS variable, settings checkbox, and tab hiding — following the exacthideSankey/hideTabularpattern. Not heavily tested but it's a mechanical copy of the existing pattern.Got rid of the description at the bottom (now behind a
showCaptionsprop, default false).I'm going to push back a little on the "one small step" button. It is a useful feature when you're really trying to see what's going on. The animation is a three-phase thing:
We can rename or restyle the button, no problem, but I don't think it's helpful to remove it entirely. But for rcvis, it's really up to you. I will say though that without that, for my purposes I'd rather stick with a version that has it.
Added 32 tests for the
graphToRCtabconverter covering: structure validation, tally correctness, eliminations, multi-winner surplus, inactive ballots, no-threshold elections, theexcludeFinalWinnerAndEliminatedCandidateflag, and the_stringifyhelper.Please do, but be aware that all we're doing is downloading a piece of code that will be cached for the lifetime of the browser instance, so it only has to happen once per browser instance. The only downloading after that will be the parameters (the color map and JSON file) to be visualized.