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

Feature request: PartialEq<[T]> for set::Slice, PartialEq<[(K, V)]> for map::Slice #375

Open
clarfonthey opened this issue Feb 14, 2025 · 4 comments · May be fixed by #376
Open

Feature request: PartialEq<[T]> for set::Slice, PartialEq<[(K, V)]> for map::Slice #375

clarfonthey opened this issue Feb 14, 2025 · 4 comments · May be fixed by #376

Comments

@clarfonthey
Copy link

This would be particularly nice for tests, so that I don't need to construct a new set/map just to assert that the values are in a particular order.

@clarfonthey clarfonthey changed the title Feature request: PartialEq<[T]> for set::Slice, PartialEq<(K, V)> for map::Slice Feature request: PartialEq<[T]> for set::Slice, PartialEq<[(K, V)]> for map::Slice Feb 14, 2025
@cuviper
Copy link
Member

cuviper commented Feb 14, 2025

You could assert!(slice.iter().eq([...])), or use itertools::assert_equal. For tests, the latter is nice because you'll get a useful debug message, including the position.

@clarfonthey
Copy link
Author

I mean, both options are fair; the latter is best for understanding the failures, but I still was thinking this would be a relatively easy solution that doesn't require any extra dependencies.

It's just hard to beat assert_eq!(set.as_slice(), [1, 2, 3]) and this notation makes extra sense considering how the set is explicitly ordered, unlike regular HashSets.

@cuviper
Copy link
Member

cuviper commented Feb 15, 2025

I just realized the iterator ways are more annoying for maps, since the items are different: &(K, V) vs. (&K, &V).

cuviper added a commit to cuviper/indexmap that referenced this issue Feb 17, 2025
```rust
impl<K: PartialEq, V: PartialEq> PartialEq<[(K, V)]> for map::Slice<K, V> {...}
impl<K: PartialEq, V: PartialEq> PartialEq<map::Slice<K, V>> for [(K, V)] {...}

impl<T: PartialEq> PartialEq<[T]> for set::Slice<T> {...}
impl<T: PartialEq> PartialEq<set::Slice<T>> for [T] {...}
```

Resolves indexmap-rs#375
@cuviper cuviper linked a pull request Feb 17, 2025 that will close this issue
@cuviper
Copy link
Member

cuviper commented Feb 17, 2025

It's just hard to beat assert_eq!(set.as_slice(), [1, 2, 3])

Playing with #376, it turns out that's not enough because the latter is an array, and you don't get automatic slice coercion here. I guess we could add impls for arrays too, but the scope is creeping...

I also wonder if we should make this more flexible, e.g. PartialEq<[U]> for Slice<T> like Vec does. That could be added later, but I think it's a minor breaking change because it affects type inference, especially for integer cases like your example.

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 a pull request may close this issue.

2 participants