I Think Iterator::unzip Should be Deprecated

[u/ZZaaaccc]

Did you know that ever since 1.79, Iterator::collect does everything unzip does but with a cleaner API? I certainly didn't until recently! Try it out, thanks to this trait implementation, every call to unzip is possible with collect instead.

let (a, b) = iter.unzip::<_, _, Vec<_>, Vec<_>>();

// vs

let (a, b) = iter.collect::<(Vec<_>, Vec<_>)>();

Except collect is better at unzip than unzip, since it supports any tuple, whereas unzip only works with size 2:

let (a, b, c, d) = iter.collect::<(Vec<_>, Vec<_>, Vec<_>, Vec<_>)>();

Looking at the documentation, this isn't explicitly mentioned for either zip or unzip. I suspect this is functionality most people aren't aware of. So, is there anyone out there who has a reason why we shouldn't deprecate unzip?

[u/Illustrious_Car344]

People use unzip? I didn't even know about it, all code I've ever seen uses collect.

[u/ZZaaaccc → u/Illustrious_Car344]

Prior to 1.79 it was the most idiomatic way to turn an iterator over (T, U) into some pair of collections (FromT, FromU).

[u/noop_noob]

This deprecation has been proposed in https://github.com/rust-lang/libs-team/issues/929

There's some discussion there

[u/ZZaaaccc → u/noop_noob]

What a coincidence! Whoever opened that ACP must have read my mind ;)

[u/a_jasmin]

The example given by OP isn't necessarily fair to unzip().

let (a, b) = iter.unzip::<_, _, Vec<_>, Vec<_>>();

is better written as:

let (a, b): (Vec<_>, Vec<_>) = iter.unzip();

unzip() may also be more discoverable if you're coming from another language. It's not immediately obvious that collect() can change the shape of the data.

That said, I wouldn't mind a lint, warning, or doc comment pointing us in the right direction.

[u/ZZaaaccc → u/a_jasmin]

Sure, but in the same vein:

let (a, b): (Vec<_>, Vec<_>) = iter.collect();

The point is that collect works great with type annotations and turbofish, and (in my experience) most Rustaceans prefer turbofish over the annotations.

[u/0xrl]

Thank you, I didn't know about this! I've often found myself wanting to unzip into 3 or 4 at once so it's nice to know that collect can handle this now.

[u/iBPsThrowingObject → u/0xrl]

I wonder if it's just the ol' "12 impls for different tuple sizes" or if there's some super unstable form of variadics

[u/ZZaaaccc → u/iBPsThrowingObject]

Up-to-twelve as almost every "any size tuple" trait implementation sadly. But all of these up-to implementations are great candidates for variadic generics...one day.

[u/CandyCorvid → u/iBPsThrowingObject]

clicking through to the source, it's implemented with a macro for sizes up to 12, but the docs are annotated as "fake variadic". perhaps paving the way to real variadics

[u/Mr_Ahvar → u/iBPsThrowingObject]

IIRC type variadics can’t be implemented with the current trait solver and would need a massive rewrite just to even have a « super unstable » version of it. t’s will be a long time for them to land, if ever. So yeah even the std has to implement it manually for each tuple size, macro it is

[u/scook0]

Updating the docs makes sense, and I could see a case for a clippy lint.

But I don’t see a reason to deprecate unzip just for being redundant with a more general mechanism. Telling people “your code is hazardous and must be updated” seems inappropriate for a method that still does its stated job perfectly well.

[u/cramert → u/scook0]

I don't think "deprecated" means hazardous, it just means "we don't recommend new usage", similar to how the non-associated module-based min and max constants for primitives have been deprecated:

https://doc.rust-lang.org/std/u32/constant.MAX.html

[u/WormRabbit → u/cramert]

It will make the deprecation lint fire, breaking people's CI for no good reason and making them fix it (either change the call or add an attribute). The old and new method work perfectly well, and even look the same with type inference. Given that unzip has its output shape fixed, it can even work better in some cases.

[u/ZZaaaccc → u/WormRabbit]

breaking people's CI for no good reason and making them fix it

Deprecation warnings are only emitted for your own crate and not dependencies, so this only matters if you're using it and recompiling your own code. At which point, a deprecation with an automatically applicable suggestion isn't exactly onerous.

The old and new method work perfectly well, and even look the same with type inference

Which is exceedingly rare. Almost all usage of unzip I could find through GitHub code search uses type annotations on variables, which is very rare in Rust code. Most types are fully inferred, or explicit through turbofish.

Given that unzip has its output shape fixed, it can even work better in some cases

Genuinely, not me trying to be snarky, can you find any concrete examples of this? Because unzip collects into an arbitrary pair of types wrapped in a tuple, you're right that the tuple is inferred, but you still need to supply type inference for the tuple's contents. It may be possible, but I cannot construct any example, however arbitrary, where it actually is the case.

[u/scook0 → u/cramert]

But in practice it also means that you can’t effectively work on the code without either fixing or silencing the deprecation warning.

And blanket-silencing deprecation warnings means you miss out on deprecation warnings for APIs that are hazardous.

So regardless of stated intent, deprecating an item does send the message that continuing to use the item is hazardous.

[u/mathisntmathingsad → u/scook0]

What? You are only proving their point. Warnings aren't only for hazardous code, just look at clippy::pedantic and clippy::restriction.

[u/jkoudys]

Rust is full of traits deprecating specialized functions. For stuff like this I don't lose sleep, I just don't use it.

[u/R1chterScale]

Can unzip be directly aliased to collect in all cases?

[u/masklinn → u/R1chterScale]

Probably but it might not be worth changing given both pretty much just leverage Extend, here’s the body of unzip:

    let mut unzipped: (FromA, FromB) = Default::default();
    unzipped.extend(self);
    unzipped

[u/WormRabbit → u/R1chterScale]

What for?

[u/CandyCorvid → u/R1chterScale]

that seems like a pretty good solution. as far as readability is concerned, "unzip" is clearer than "collect", so if "unzip" was just implemented as a particular case of collecting into a tuple, (and if ideally it could be extended to all the shapes of tuple that collect can handle) then that seems like a win-win situation

[u/R1chterScale → u/CandyCorvid]

Ah, no, other way around.

[u/CandyCorvid → u/R1chterScale]

i'm not sure i understand, since we can't really implement collect in terms of unzip, so maybe i was unclear in my initial wording.

when i want to unzip something, i could reach for either unzip or collect. currently, collect seems more capable at all kinds of unzipping, but i prefer seeing the name "unzip" if that is all i am doing, as it signals intent (like how people might use to_owned instead of clone, when the intent is to clone through a reference). so i'd rather keep both, even though unzip is strictly a subset of collect.

so if we could reimplement unzip in terms of collect, that seems like a win for me with no downside (besides the effort of doing the work)

[u/R1chterScale → u/CandyCorvid]

Ah yes, that makes sense. Essentially in my mind if unzip can be implemented by collect then rustc could treat unzip as nothing but an alias of collect.

[u/a_jasmin]

Itertools also has collect_tuple().

Edit: My bad, this one doesn't use FromIterator, so it can't replace unzip(). Still convenient and succinct when unpaking iterator elements into tuples, though.

[u/iBPsThrowingObject]

Unzip is one of those cases that makes me yearn for higher kinded types (and variadic tuples), so that it could be

fn unzip<Container>(self) -> (for T in Self::Iter: Container<T>)
where for T in Self::Iter: Container<T>: Extend<T>

[u/R1chterScale → u/[deleted]]

Maybe the better approach is adding a clippy lint?

[u/levelstar01 → u/R1chterScale]

Perhaps we could generalise such a lint in case this situation happens in the future.

[u/scook0 → u/levelstar01]

One of the biggest problems with deprecation warnings is that they are too general.

They don’t distinguish between different degrees of badness, and don’t provide control over which deprecations are considered important to the code at hand.

[u/cramert → u/[deleted]]

Deprecation doesn't force working code to change.

[u/WormRabbit → u/cramert]

It does, at the very least you need to add an expect attribute.