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

100 persist irvballotinterpretation #155

Merged
merged 22 commits into from
Jul 23, 2024
Merged

Conversation

vteague
Copy link
Member

@vteague vteague commented Jul 21, 2024

This adds a new database table for persisting the valid interpretations of invalid IRV ballots. This is not actually used in the RLA computation, but is reported in its own report (also part of this PR).

Note this only deals with the UPLOAD Cvrs - the audit CVRs will need to be added, too, but that is in a separate card.

@vteague vteague linked an issue Jul 21, 2024 that may be closed by this pull request
@Version
@Column(name = "version", updatable = false, nullable = false)
private long version;

Copy link
Member

Choose a reason for hiding this comment

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

If this is meant to be a non-updatable data structure, then it is odd to have non final public attributes. I would make them private and just have getters for the required fields.

*/
public IRVBallotInterpretation() {
}

Copy link
Member

Choose a reason for hiding this comment

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

Worth adding in the comments that we are creating a record of an interpretation of an invalid IRV vote.

* @param imprintedId the imprinted ID, i.e. tabulator_id-batch_id-record_id.
* @param rawChoices the (invalid) raw IRV choices, e.g. [Bob(1),Alice(3),Chuan(4)].
* @param orderedChoices the way colorado-rla interpreted the raw choices, as an order list of names.
*/
Copy link
Member

Choose a reason for hiding this comment

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

Does line 99 fall within the line length guideline?

* during the auditing steps, a record is made of the CVR ID, the county, contest name, raw vote,
* and our interpretation of that vote as an ordered list of candidate names.
* This is output as a report called IRVBallotInterpretationReport.
*/
Copy link
Member

Choose a reason for hiding this comment

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

Check whether we need this to implement the Serializable interface in addition to PersistentEntity. If not, some of the methods can be removed if we don't implement it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I got rid of Serizalizable but unfortunately those 3 overridden messages are still needed for PersistentEntity.

this.rawChoices = rawChoices;
this.interpretation = orderedChoices;
}

Copy link
Member

Choose a reason for hiding this comment

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

"Output details of the stored IRV vote interpretation as a String ..."


// Make the ranked_ballot_interpretation report.
final Map<String, String> files = ExportQueries.sqlFiles();
final ByteArrayOutputStream os = new ByteArrayOutputStream();
Copy link
Member

Choose a reason for hiding this comment

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

Check whether non-final declared variables can be made final.

@@ -92,6 +102,14 @@ public class GetAssertionsTests {
*/
Copy link
Member

Choose a reason for hiding this comment

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

The baseURL attribute is just labelled 'static' without a private/public etc.


/**
* Test that a correct ranked vote interpretation report is produced, for a variety of IRV-related
* parsing examples, including:
Copy link
Member

Choose a reason for hiding this comment

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

will -> with


/**
* Class-wide logger
*/
Copy link
Member

Choose a reason for hiding this comment

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

Generally we don't have the au.org. etc prefix for the Class.class statement here.


DominionCVRExportParser parser = new DominionCVRExportParser(reader, fromString("Gilpin"), blank, true);
assertTrue(parser.parse().success);

Copy link
Member

Choose a reason for hiding this comment

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

Should q and cvr be final?

@vteague vteague merged commit 082738a into main Jul 23, 2024
1 check passed
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.

Persist IRVBallotInterpretation
2 participants