Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates to the per-slice CSV #872

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

artoonie
Copy link
Collaborator

@artoonie artoonie commented Jul 10, 2024

Closes #871. Makes several changes as outlined there.

@artoonie artoonie added the WIP label Jul 10, 2024
@artoonie artoonie force-pushed the feature/issue-871_tabulate-by-csv-updates branch from 2e2ae51 to ccc2def Compare July 10, 2024 19:56
@artoonie artoonie removed the WIP label Jul 10, 2024
@artoonie artoonie force-pushed the feature/issue-871_tabulate-by-csv-updates branch from ccc2def to 4445a3c Compare July 10, 2024 20:55
@artoonie artoonie force-pushed the feature/issue-871_tabulate-by-csv-updates branch from 5afe50b to eb4c928 Compare September 4, 2024 16:13
if (!isNullOrBlank(sliceId)) {
// Only silces print the slice information
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Only silces print the slice information
// Only slices print the slice information

@yezr
Copy link
Collaborator

yezr commented Sep 6, 2024

Testing

  • confirmed candidate order in tabulate by precinct and tabulate by batch
  • confirmed elect/eliminate in tabulate by precinct and tabulate by batch
  • confirmed removal of both thresholds in tabulate by precinct and tabulate by batch
  • confirmed additional text at the bottom of summary.csv for precinct files and batch files

I don't see any tabulate by batch files currently in tests and I think we should include at least one. I tried to do this by using the contest that had already changed (Minneapolis Mayor 2013), turning on "Tabulate by Batch", and set the Batch Column setting in the CVR config to 1 in order to just re-use the precinct ids as batch ids. That way they should be equivalent except for the minor tabulate by batch/precinct differences. I was then gonna check in one batch summary file, the updated config and change the equivalent test to expect more files.

However, when running the tabulation with Precinct Column and Batch Column set to 1 I got warnings that every row didn't have a batch id. I looked it the code and that's because the StreamingCvrReader only allows a cell to be one of precinct id, batch id, ballot id or an actual ranking - never more than one of those things.

image

I changed Batch Column to 4 to use the extra count column that ES&S CVRs spit out. Effectively, every batch id = 1. That resulted in this file that exactly matches the full contest summary.csv except for the tabulate by batch specific edits, as expected.
2024-09-06_15-15-03_1_batch_summary.csv

I think we need to check one batch slice file in. What I did here seems kind of clunky. We could use the smaller contest I made to test tabulate by batch earlier
CSV CVRs - tab by batch test.csv

Other than adding in at least one batch summary the changes LGTM! Ready for a live code review!

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.

Tabulate by slice updates
3 participants