biqqles / dataclassy

A fast and flexible reimplementation of data classes
https://pypi.org/project/dataclassy
Mozilla Public License 2.0
82 stars 9 forks source link

Add cmp_internals + tests #54

Closed thisisrandy closed 3 years ago

thisisrandy commented 3 years ago

This is an implementation of the first idea to address #53. There are two things I don't like about it:

  1. Even though the new option is called cmp_internals, it also affects iteration, since eq, order, and iter all make use of __tuple__ under the hood. I think a sensible interface would be to also have iter_internals to control iter behavior separately, but that involves either storing two versions of __tuple__ when the two options don't agree or filtering a single version containing internals on the fly as necessary, both of which have overhead. Thoughts would be appreciated.
  2. This is a breaking change (you'll notice I had to set cmp_internals=False to get all the tests using Epsilon to work). The easy fix is just to make it False by default, but as I argue in #53, I believe it should be True by default in order to agree with dataclasses, not to mention your own docs :grin:. Again, thoughts would be appreciated.
thisisrandy commented 3 years ago

Closing per discussion in #53

biqqles commented 3 years ago

I was about to say your tests are likely still useful so please keep the branch, but it seems GitHub will remember the diff here, happily.

thisisrandy commented 3 years ago

I just wanted to start from scratch in a new and appropriately-named branch. Sorry for the confusion. Please see #56