Implement PrimaryKey for generic (T, U, V) triplet #210
Conversation
| impl<'a, T: PrimaryKey<'a> + Prefixer<'a>, U: PrimaryKey<'a> + Prefixer<'a>, V: PrimaryKey<'a>> | ||
| PrimaryKey<'a> for (T, U, V) | ||
| { | ||
| type Prefix = T; |
There was a problem hiding this comment.
Good catch. Wouldn't we want to consider both prefixes as valid? I mean, (T, U) is a prefix, but wouldn't T be another valid one?
There was a problem hiding this comment.
Sure, but look how this is used.
Maybe we need to go through this all and check the usage and document it better.
I don't have the head space to go deep in here now. Best is to leave these PRs open and I will review and merge them (or comment on them) when I have the time to check all the details.
There was a problem hiding this comment.
Sure, but look how this is used.
This change will force me to change the implementation of MultiIndex, currently in review (#211).
But that's OK, as this is the right way to implement Prefix for these types. I'll do the changes, and will have a better look at the trade-offs.
Maybe we need to go through this all and check the usage and document it better.
Sure. I'll document the final format properly.
I don't have the head space to go deep in here now. Best is to leave these PRs open and I will review and merge them (or comment on them) when I have the time to check all the details.
No worries. I'll leave the MultiIndex PR open. That's the only place we're actually using / requiring this.
|
I re-read the issue and saw:
I understand only supporting |
OK. I'll create a small PR with the tests. |
Merged because this was based on the original code / logic for |
Closes #197.
PrimaryKeyfor nested((T, U), V)tuples cannot at the moment be defined univocally. They can be defined, but only in a way that deserializes to(T, U, V), i.e. the nesting is lost when serializing.They are not needed / required at this point, so I think it's OK the leave them undefined for the moment.