Introduce the partition_dedup/by/by_key methods for slices

[Kerollmops]

This PR propose to add three methods to the slice type, the partition_dedup, partition_dedup_by and partition_dedup_by_key. The two other methods are based on slice::partition_dedup_by.

These methods take a mutable slice, deduplicates it and moves all duplicates to the end of it, returning two mutable slices, the first containing the deduplicated elements and the second all the duplicates unordered.

let mut slice = [1, 2, 2, 3, 3, 2];

let (dedup, duplicates) = slice.partition_dedup();

assert_eq!(dedup, [1, 2, 3, 2]);
assert_eq!(duplicates, [3, 2]);

The benefits of adding these methods is that it is now possible to:

• deduplicate a slice without having to allocate and possibly clone elements on the heap, really useful for embedded stuff that can't allocate for example.
• not loose duplicate elements, because, when using Vec::dedup, duplicates elements are dropped. These methods add more flexibillity to the user.

Note that this is near a copy/paste of the Vec::dedup_by function, once this method is stable the goal is to replace the algorithm in Vec by the following.

pub fn Vec::dedup_by<F>(&mut self, same_bucket: F)
    where F: FnMut(&mut T, &mut T) -> bool
{
    let (dedup, _) = self.as_mut_slice().partition_dedup_by(same_bucket);
    let len = dedup.len();
    self.truncate(len);
}

[rust-highfive]

r? @shepmaster

(rust_highfive has picked a reviewer for you, use r? to override)

[shepmaster]

once this method is stable

Internal code is allowed to use unstable features. I'd expect that switching Vec to use this code earlier would be better to get earlier usage of it.

[shepmaster]

🤔

Thinking on this... I really want to call the two returned slices "buckets" because I think of this algorithm as "assigning values in the slice to one bucket or the other", based on the predicate of "is this equal to the previous one".

Having said that, I wonder if this function could/should more accurately be referred to as a type of partition. The return type certainly matches much more closely with Iterator::partition.

Something like partition_dedup?

[Kerollmops]

retain_dedup was something that I choose too quickly. I prefer partition_dedup too because currently the retain word is used for behavior that doesn't stick to the one I am trying to implement (e.g. Vec::retain).

[shepmaster]

This feels uncontroversial enough to me. You are really just moving some code from Vec<T> to [T]. I think the next step is to open a tracking issue and then update the unstable attributes here with the appropriate number.

[Kerollmops]

Whoops, was it to me to open this tracking issue ?

[shepmaster]

Whoops, was it to me to open this tracking issue ?

I think that's fine.

[shepmaster]

The last thing I'd suggest before merging is squashing the commits to remove the back and forth of the renaming and doc tweaks, etc.

My suggestion would be squash everything except for "Make the Vec::dedup method use slice::retain_dedup internally". This PR would then end up with two commits: introducing the slice method (temporarily copying most of the implementation from Vec) and then DRYing it up by changing Vec::dedup to use the slice method.

[leonardo-m]

Could you please show use cases for the "duplicates" part of the result? They should justify the added complexity of not just returning the deduped part.

Also, in most cases what I need is to dedup sorted iterables and have an iterable as result (as the uniq method in D language std lib does: https://dlang.org/phobos/std_algorithm_iteration.html#uniq ).

[shepmaster]

the added complexity of not just returning the deduped part.

I don't know that I'd agree that there's much complexity added by doing so. The algorithm naturally results in the two pieces existing.

That being said, it could be argued that by returning both pieces, it may restrict us on other implementations of algorithms that could also be used.

dedup sorted iterables and have an iterable as result

You are looking for Itertools::dedup

[leonardo-m]

I don't know that I'd agree that there's much complexity added by doing so.

Future Rust programmers that in their code will want to use that function will need to remember or look in the docs or ask the IDE to know what of the two results is the one they want, and they will have to write this, as done in the first post of this thread:

let (dedup, _) =

Every small thing like this adds up, and makes writing code slower. That's why you need good use cases to justify increased API complexity. Python designers know this well.

You are looking for Itertools::dedup

I think that such lazy iterator is more fundamental and more commonly useful than a slice dedup, and I'd like it in the std library more than slice dedup.

[Kerollmops]

First I asked myself the same thing: "Why didn't we return the deduplicated slice part?".

I think that if we do that so the fonction must be named dedup that is the same name as the one in Vec but with a different behavior. The slice one returns the slice part that contains deduplicated elements and the Vec one change the value in place without returning anything. So there is two different behavior for the same name.

Secondly what happens to the users who wants to retrieve the duplicates? They will have to compute the slice part that contains all the duplicates: this is error prone.

let mut slice = [1, 2, 2, 3, 3, 2];

let dedup = slice.partition_dedup();
assert_eq!(dedup, [1, 2, 3, 2]);

let duplicates = slice[dedup.len()..];
assert_eq!(duplicates, [3, 2]);

A solution could be to name the function dedup and mark it as #[must_use], warning the user when he forget to use the returned deduplicated slice.

Another solution could be to drop the end (i.e. the duplicated elements) by changing the bounds of self, returning nothing like the Vec::dedup function already do. So the way to retrieve the duplicated elements is the same as above but we need to duplicate the slice before. I prefer this solution.

let slice = &mut [1, 2, 2, 3, 3, 2];
let slice2 = slice;

slice.dedup();
assert_eq!(slice, &mut [1, 2, 3, 2]);

let duplicates = &slice2[dedup.len()..];
assert_eq!(duplicates, &[3, 2]);

EDIT: The last solution is not something that is possible, the &mut [] is passed to the dedup function by copy. The dedup function gets a fat pointer, can change its bounds but these modifications are not visible to the caller.

[shepmaster]

I think that such lazy iterator is more fundamental and more commonly useful than a slice dedup, and I'd like it in the std library more than slice dedup.

They aren't exclusive propositions; I see no reason you shouldn't propose inclusion of Iterator::dedup separately.

I'd argue that the proposed method would be used at a lower level than the iterator one because it allows getting a collection of unique values without allocation.

Future Rust programmers that in their code will want to use that function

Ah, you are referring to complexity of usage, not complexity of implementation. Such a distinction is important to highlight up front.

That being said, I think that by using partition in the method name, users will already be prepared for a 2-element tuple as return value, similar to Iterator::partition.

Since these are unstable methods, I'd be in favor of merging them such that they return the tuple. This allows users to use maximum possible functionality. I think that deciding if it should be stabilized as such would be an excellent question for the tracking issue and if no one uses the second value, then we simplify the signature.

[shepmaster]

@bors r+ rollup

[bors]

📌 Commit 6240a699260b78ead5fb5204ae1a88007922e9ab has been approved by shepmaster

[shepmaster]

an excellent question for the tracking issue

@leonardo-m to be clear, I think that you should make sure that your point-of-view is represented on the tracking issue as you are the best person to make that comment. 😉

[bors]

☔ The latest upstream changes (presumably #54457) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

☔ The latest upstream changes (presumably #53508) made this pull request unmergeable. Please resolve the merge conflicts.

[shepmaster]

@bors r+ rollup

[bors]

📌 Commit d560292 has been approved by shepmaster

[bors]

⌛ Testing commit d560292 with merge cd6812e1d19fa95ac27c8936158ed3a447367579...

[bors]

💔 Test failed - status-travis

[rust-highfive]

The job dist-i686-apple of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[00:04:00]       Memory: 8 GB
[00:04:00]       Boot ROM Version: VMW71.00V.0.B64.1704110547
[00:04:00]       Apple ROM Info: [MS_VM_CERT/SHA1/27d66596a61c48dd3dc7216fd715126e33f59ae7]Welcome to the Virtual Machine
[00:04:00]       SMC Version (system): 2.8f0
[00:04:00]       Serial Number (system): VMSUw/Gm0wVB
[00:04:00] 
[00:04:00] hw.ncpu: 4
[00:04:00] hw.byteorder: 1234
[00:04:00] hw.memsize: 8589934592
---
uploading "cd6812e1d19fa95ac27c8936158ed3a447367579/clippy-nightly-i686-apple-darwin.tar.xz" with {:content_type=>"application/x-xz", :acl=>"public-read"}
uploading "cd6812e1d19fa95ac27c8936158ed3a447367579/clippy-nightly-i686-apple-darwin.tar.gz" with {:content_type=>"application/gzip", :acl=>"public-read"}
uploading "cd6812e1d19fa95ac27c8936158ed3a447367579/cargo-nightly-i686-apple-darwin.tar.xz" with {:content_type=>"application/x-xz", :acl=>"public-read"}
uploading "cd6812e1d19fa95ac27c8936158ed3a447367579/cargo-nightly-i686-apple-darwin.tar.gz" with {:content_type=>"application/gzip", :acl=>"public-read"}
/Users/travis/.rvm/gems/ruby-2.4.2/gems/aws-sdk-resources-2.11.136/lib/aws-sdk-resources/services/s3/multipart_file_uploader.rb:81:in `abort_upload': multipart upload failed: Encountered a `SocketError` while attempting to connect to: (Aws::S3::MultipartUploadError)
  https://rust-lang-ci2.s3.us-west-1.amazonaws.com/rustc-builds/cd6812e1d19fa95ac27c8936158ed3a447367579/rust-std-nightly-i686-apple-darwin.tar.xz?partNumber=2&uploadId=1gWMYLcN5MIIKc.1dEXzTeFfCK2gGZ0MzdtyCGCBw3KAIybK3Q_xqUMRGdkyIbMkN.64ZjIlkmFnpLkWnCyTBIGuk_JSNi11RphiUyTQ6b7HJpqbRAR.7CNH880JEZms
This is typically the result of an invalid `:region` option or a
poorly formatted `:endpoint` option.
* Avoid configuring the `:endpoint` option directly. Endpoints are constructed
  from the `:region`. The `:endpoint` option is reserved for connecting to
  non-standard test endpoints.
* Not every service is available in every region.
* Never suffix region names with availability zones.
  Use "us-east-1", not "us-east-1a"
Known AWS regions include (not specific to this service):
ap-northeast-1
ap-northeast-2
ap-south-1
ap-southeast-1
ap-southeast-2
ca-central-1
eu-central-1
eu-west-1
eu-west-2
eu-west-3
sa-east-1
us-east-1
us-east-2
us-west-2
cn-north-1
cn-north-1
cn-northwest-1
us-gov-west-1
; Encountered a `SocketError` while attempting to connect to:
  https://rust-lang-ci2.s3.us-west-1.amazonaws.com/rustc-builds/cd6812e1d19fa95ac27c8936158ed3a447367579/rust-std-nightly-i686-apple-darwin.tar.xz?partNumber=7&uploadId=1gWMYLcN5MIIKc.1dEXzTeFfCK2gGZ0MzdtyCGCBw3KAIybK3Q_xqUMRGdkyIbMkN.64ZjIlkmFnpLkWnCyTBIGuk_JSNi11RphiUyTQ6b7HJpqbRAR.7CNH880JEZms
This is typically the result of an invalid `:region` option or a
poorly formatted `:endpoint` option.
* Avoid configuring the `:endpoint` option directly. Endpoints are constructed
  from the `:region`. The `:endpoint` option is reserved for connecting to
  non-standard test endpoints.
* Not every service is available in every region.
* Never suffix region names with availability zones.
  Use "us-east-1", not "us-east-1a"
Known AWS regions include (not specific to this service):
ap-northeast-1
ap-northeast-2
ap-south-1
ap-southeast-1
ap-southeast-2
ca-central-1
eu-central-1
eu-west-1
eu-west-2
eu-west-3
sa-east-1
us-east-1
us-east-2
us-west-2
cn-north-1
cn-north-1
cn-northwest-1
us-gov-west-1
; Encountered a `SocketError` while attempting to connect to:
  https://rust-lang-ci2.s3.us-west-1.amazonaws.com/rustc-builds/cd6812e1d19fa95ac27c8936158ed3a447367579/rust-std-nightly-i686-apple-darwin.tar.xz?partNumber=4&uploadId=1gWMYLcN5MIIKc.1dEXzTeFfCK2gGZ0MzdtyCGCBw3KAIybK3Q_xqUMRGdkyIbMkN.64ZjIlkmFnpLkWnCyTBIGuk_JSNi11RphiUyTQ6b7HJpqbRAR.7CNH880JEZms
This is typically the result of an invalid `:region` option or a
poorly formatted `:endpoint` option.
* Avoid configuring the `:endpoint` option directly. Endpoints are constructed
  from the `:region`. The `:endpoint` option is reserved for connecting to
  non-standard test endpoints.
* Not every service is available in every region.
* Never suffix region names with availability zones.
  Use "us-east-1", not "us-east-1a"
Known AWS regions include (not specific to this service):
ap-northeast-1
ap-northeast-2
ap-south-1
ap-southeast-1
ap-southeast-2
ca-central-1
eu-central-1
eu-west-1
eu-west-2
eu-west-3
sa-east-1
us-east-1
us-east-2
us-west-2
cn-north-1
cn-north-1
cn-northwest-1
us-gov-west-1
; Encountered a `SocketError` while attempting to connect to:
  https://rust-lang-ci2.s3.us-west-1.amazonaws.com/rustc-builds/cd6812e1d19fa95ac27c8936158ed3a447367579/rust-std-nightly-i686-apple-darwin.tar.xz?partNumber=8&uploadId=1gWMYLcN5MIIKc.1dEXzTeFfCK2gGZ0MzdtyCGCBw3KAIybK3Q_xqUMRGdkyIbMkN.64ZjIlkmFnpLkWnCyTBIGuk_JSNi11RphiUyTQ6b7HJpqbRAR.7CNH880JEZms
This is typically the result of an invalid `:region` option or a
poorly formatted `:endpoint` option.
* Avoid configuring the `:endpoint` option directly. Endpoints are constructed
  from the `:region`. The `:endpoint` option is reserved for connecting to
  non-standard test endpoints.
* Not every service is available in every region.
* Never suffix region names with availability zones.
  Use "us-east-1", not "us-east-1a"
Known AWS regions include (not specific to this service):
ap-northeast-1
ap-northeast-2
ap-south-1
ap-southeast-1
ap-southeast-2
ca-central-1
eu-central-1
eu-west-1
eu-west-2
eu-west-3
sa-east-1
us-east-1
us-east-2
us-west-2
cn-north-1
cn-north-1
cn-northwest-1
us-gov-west-1
; Encountered a `SocketError` while attempting to connect to:
  https://rust-lang-ci2.s3.us-west-1.amazonaws.com/rustc-builds/cd6812e1d19fa95ac27c8936158ed3a447367579/rust-std-nightly-i686-apple-darwin.tar.xz?partNumber=5&uploadId=1gWMYLcN5MIIKc.1dEXzTeFfCK2gGZ0MzdtyCGCBw3KAIybK3Q_xqUMRGdkyIbMkN.64ZjIlkmFnpLkWnCyTBIGuk_JSNi11RphiUyTQ6b7HJpqbRAR.7CNH880JEZms
This is typically the result of an invalid `:region` option or a
poorly formatted `:endpoint` option.
* Avoid configuring the `:endpoint` option directly. Endpoints are constructed
  from the `:region`. The `:endpoint` option is reserved for connecting to
  non-standard test endpoints.
* Not every service is available in every region.
* Never suffix region names with availability zones.
  Use "us-east-1", not "us-east-1a"
Known AWS regions include (not specific to this service):
ap-northeast-1
ap-northeast-2
ap-south-1
ap-southeast-1
ap-southeast-2
ca-central-1
eu-central-1
eu-west-1
eu-west-2
eu-west-3
sa-east-1
us-east-1
us-east-2
us-west-2
cn-north-1
cn-north-1
cn-northwest-1
us-gov-west-1
; Encountered a `SocketError` while attempting to connect to:
  https://rust-lang-ci2.s3.us-west-1.amazonaws.com/rustc-builds/cd6812e1d19fa95ac27c8936158ed3a447367579/rust-std-nightly-i686-apple-darwin.tar.xz?partNumber=6&uploadId=1gWMYLcN5MIIKc.1dEXzTeFfCK2gGZ0MzdtyCGCBw3KAIybK3Q_xqUMRGdkyIbMkN.64ZjIlkmFnpLkWnCyTBIGuk_JSNi11RphiUyTQ6b7HJpqbRAR.7CNH880JEZms
This is typically the result of an invalid `:region` option or a
poorly formatted `:endpoint` option.
* Avoid configuring the `:endpoint` option directly. Endpoints are constructed
  from the `:region`. The `:endpoint` option is reserved for connecting to
  non-standard test endpoints.
* Not every service is available in every region.
* Never suffix region names with availability zones.
  Use "us-east-1", not "us-east-1a"
Known AWS regions include (not specific to this service):
ap-northeast-1
ap-northeast-2
ap-south-1
ap-southeast-1
ap-southeast-2
ca-central-1
eu-central-1
eu-west-1
eu-west-2
eu-west-3
sa-east-1
us-east-1
us-east-2
us-west-2
cn-north-1
cn-north-1
cn-northwest-1
us-gov-west-1
; Encountered a `SocketError` while attempting to connect to:
  https://rust-lang-ci2.s3.us-west-1.amazonaws.com/rustc-builds/cd6812e1d19fa95ac27c8936158ed3a447367579/rust-std-nightly-i686-apple-darwin.tar.xz?partNumber=9&uploadId=1gWMYLcN5MIIKc.1dEXzTeFfCK2gGZ0MzdtyCGCBw3KAIybK3Q_xqUMRGdkyIbMkN.64ZjIlkmFnpLkWnCyTBIGuk_JSNi11RphiUyTQ6b7HJpqbRAR.7CNH880JEZms
This is typically the result of an invalid `:region` option or a
poorly formatted `:endpoint` option.
* Avoid configuring the `:endpoint` option directly. Endpoints are constructed
  from the `:region`. The `:endpoint` option is reserved for connecting to
  non-standard test endpoints.
* Not every service is available in every region.
* Never suffix region names with availability zones.
  Use "us-east-1", not "us-east-1a"
Known AWS regions include (not specific to this service):
ap-northeast-1
ap-northeast-2
ap-south-1
ap-southeast-1
ap-southeast-2
ca-central-1
eu-central-1
eu-west-1
eu-west-2
eu-west-3
sa-east-1
us-east-1
us-east-2
us-west-2
cn-north-1
cn-north-1
cn-northwest-1
us-gov-west-1
 from /Users/travis/.rvm/gems/ruby-2.4.2/gems/aws-sdk-resources-2.11.136/lib/aws-sdk-resources/services/s3/multipart_file_uploader.rb:70:in `upload_parts'
 from /Users/travis/.rvm/gems/ruby-2.4.2/gems/aws-sdk-resources-2.11.136/lib/aws-sdk-resources/services/s3/multipart_file_uploader.rb:44:in `upload'
 from /Users/travis/.rvm/gems/ruby-2.4.2/gems/aws-sdk-resources-2.11.136/lib/aws-sdk-resources/services/s3/file_uploader.rb:32:in `upload'
 from /Users/travis/.rvm/gems/ruby-2.4.2/gems/aws-sdk-resources-2.11.136/lib/aws-sdk-resources/services/s3/object.rb:252:in `upload_file'
 from /Users/travis/.rvm/gems/ruby-2.4.2/gems/dpl-s3-1.10.1/lib/dpl/provider/s3.rb:106:in `block (2 levels) in upload_multithreaded'
failed to deploy
failed to deploy
No output has been received in the last 30m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received
The build has been terminated

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

[shepmaster]

@bors retry