Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upGitHub is where the world builds software
Millions of developers and companies build, ship, and maintain their software on GitHub — the largest and most advanced development platform in the world.
Implement PartialEq/Eq for SliceInfo #689
Conversation
|
looks good. If the indices are equal, then D1::NDIM and D2::NDIM should be too, and don't need to be checked |
| @@ -295,6 +295,25 @@ pub struct SliceInfo<T: ?Sized, D: Dimension> { | |||
| indices: T, | |||
| } | |||
|
|
|||
| impl<T1, D1, T2, D2> PartialEq<SliceInfo<T2, D2>> for SliceInfo<T1, D1> | |||
jturner314
Sep 4, 2019
•
Member
Why are D1 and D2 allowed to be different types? If D1 != D2, the SliceInfo instances are guaranteed to be unequal. I would think it would be more useful to catch that case at compile time instead of waiting until runtime.
jturner314
Sep 4, 2019
Member
Oh, right, I forgot that it's possible to create SliceInfo instances without using the s![] macro.
There's still some question in my mind, though, whether SliceInfo<T1, D1> and SliceInfo<T2, D2> should be considered equal (if their contents are equal) even if D1 and D2 are different. Both of these interpretations make sense to me:
-
They should be equal because they will slice arrays in the same way (same indices and resulting shape).
-
They should be unequal because the arrays resulting from slicing will have a different dimension types.
Since both interpretations make sense, I'd prefer to avoid the ambiguity by requiring D1 and D2 to be the same in the PartialEq impl. If someone wants to compare the contents of SliceInfo instances with different dimension types, they can just use info1.as_ref() == info2.as_ref().
bluss
Sep 16, 2019
Member
I wonder why we encourage this constant .as_ref() calling. ("This is not how conversion traits are supposed to be used, and it's fragile" is the short story).
We should add a method to them, if it's intended that you convert them like this in open code (not as part of converting function arguments).
|
@aldanor I think you'd use this code in your project? can you tell us what choice for the Dyn question makes sense for the use case you've seen? |
Noticed that
SliceInfodoesn't implement equality ops, has it been overlooked?Not sure about the
Dparameter, if the indices are the same, do we consider slice info to be equal, regardless of dimension types?