✨ Misc issues with manifest.py classes

[JohnLCaron]

Is there an existing issue for this?

• I have searched the existing issues

Suggestion

1. Manifest
   does not use candidates in its crypto_hash(). Seems like it should.

    def crypto_hash(self) -> ElementModQ:
        """
        Returns a hash of the metadata components of the election
        """
        hash = hash_elems(
            self.election_scope_id,
            str(self.type.name),
            to_iso_date_string(self.start_date),
            to_iso_date_string(self.end_date),
            self.name,
            self.contact_information,
            self.geopolitical_units,
            self.parties,
            self.contests,
            self.ballot_styles,
        )
        log_debug(f"{self.__class__} : crypto_hash: {hash.to_hex()}")
        return hash

2. ContestDescription has votes_allowed as Optional. if thats needed, would be good to document what that means. Otherwise, recommend to make it non-optional.

3. Candidate has is_write_in as Optional. Recommend having a non-optional bool for clarity.

4. CandidateContestDescription, ReferendumContestDescription seem unclear as when and why to use. Perhaps withdraw these from the reference library for now, until they are ready to be developed further?

5. ContestDescriptionWithPlaceholders and InternalManifest: not clear if this is really used, or needed, or what its status is. Perhaps its a start of an n-of-m implementation extension? Is it tested? It should probably be split into its own file, so that the Manifest classes can just be the election description, that election officials would understand, and not implementation details.

Possible Implementation

No response

Anything else?

No response

[JohnLCaron]

Ok, I see that ContestDescriptionWithPlaceholders and InternalManifest are needed. Still, pulling them out of manifest.py seems like a good way to separate concerns.

[keithrfung]

@rc These issues should definitely be isolated into their own issues. Reference this issue from within them. Will answer here but this should stay in triage until addressed.

1. Valid point. Remember this trickles into electionguard-cpp as well. Has to be fixed in both places. These type of issues are a strong example of why electionguard should dictate how things are serialized, so that these issues can exist in a common place.
2. Agreed.
3. Agreed. I don't believe this field is in the Google Civic spec so this is something that should be shiftable. It would also impact cpp though.
4. I agree. These types add little value but make the code more complicated.
5. I'm in agreement with the file/folder restructure as a whole. I think this is ElectionGuard 2.0 material personally. There are some overall architecture and organization restructuring needed.

[keithrfung]

Took a bit but I have all these referenced in new issues.

[rc-ms]

thanks Keith I agree generally and will weigh in on each issue. thanks for suggesting!