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

Version 2.1.6! New custom serializer. #11

Merged

Conversation

larryhastings
Copy link
Owner

Bugfix: the hashed_ballots_tiebreaker assumed
that marshal.dumps produced an identical bytes
string given identical inputs. This is not true!
We also apparently can't rely on pickle.dumps
to be deterministic. So, starvote now has
its own bespoke--and completely deterministic--simple binary serializer, called starvote_custom_serializer. It's tailor-made for the needs of starvote and
isn't useful for anybody else. But it does guarantee that hashed_ballots_tiebreaker will now produce
identical results across all supported Python
versions, across all architectures, regardless of
optimization level.

Thanks to Petr Viktorin for the bug report! Fixes #10.

Bugfix: the hashed_ballots_tiebreaker assumed
that marshal.dumps produced an identical bytes
string given identical inputs.  This is not true!
We also apparently can't rely on pickle.dumps
to be deterministic.  So, starvote now has
its own bespoke--and completely deterministic--simple
binary serializer, called starvote_custom_serializer.
It's tailor-made for the needs of starvote and
isn't useful for anybody else.  But it does guarantee
that hashed_ballots_tiebreaker will now produce
identical results across all supported Python
versions, across all architectures, regardless of
optimization level.

Thanks to Petr Viktorin for the bug report!  Fixes #10.
Copy link

@encukou encukou left a comment

Choose a reason for hiding this comment

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

LGTM, but it needs a few documentation changes, and perhaps dropping a limitation.

Added starvote_custom_deserializer, which allows us to
test that starvote_custom_serializer is working correctly.
(Not that it matters--it just needs to produce *something*
that's approximately unique-ish for these ballots.  But now
we know for certain!)
@larryhastings
Copy link
Owner Author

I went ahead and wrote a custom deserializer to match my custom serializer. Now we can be reasonably certain that the custom serializer is working as intended, thank goodness!

@encukou
Copy link

encukou commented Dec 16, 2024

My pedantic jib¹⁾ remains, as this bit of README is out of date:

The candidate can be any hashable Python value;

The serializer demands strings.

¹⁾ (a word I looked up but still don't quite get)

Raise exceptions if they aren't, and test those exceptions too.
Holding steady at 100% coverage.
@larryhastings
Copy link
Owner Author

"the cut of your jib" is an old-timey expression meaning "your general style / demeanor". The only way you ever hear it is the sentence "I like the cut of your jib!", which means "I like your style!" (I don't understand the literal phrase, it sounds like you're saying you like how they styled one particular sail on their ship.)

The serializer demands strings.

Good point! I'll mention that. Hopefully that's a restriction folks can live with.

I believe the rest of the API is still sufficiently agnostic that any hashable value will work. You're only restricted to candidates being strings if you use the hashable_ballots_tiebreaker.

@encukou
Copy link

encukou commented Dec 16, 2024

Ah! Silly me, looking up individual words in an English phrase. I should know better than to do that :)

LGTM.

@larryhastings larryhastings merged commit c277469 into master Dec 16, 2024
11 checks passed
@larryhastings larryhastings deleted the add_custom_serializer_for_hashed_ballot_tiebreaker branch December 16, 2024 14:04
@larryhastings
Copy link
Owner Author

Thanks, Petr! I appreciate all your smart feedback.

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.

marshal.dumps does not always produce identical results given inputs that compare equal
2 participants