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

Tracking Issue for Iterator::find_map #49602

Closed
KodrAus opened this issue Apr 2, 2018 · 14 comments
Closed

Tracking Issue for Iterator::find_map #49602

KodrAus opened this issue Apr 2, 2018 · 14 comments
Labels
A-iterators Area: Iterators C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@KodrAus
Copy link
Contributor

KodrAus commented Apr 2, 2018

Iterator::find_map, like filter_map but for the first matching item in the iterator.

Implemented in #49098

cc @matklad

@KodrAus KodrAus added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels Apr 2, 2018
@Centril Centril added the A-iterators Area: Iterators label Apr 3, 2018
@Amanieu
Copy link
Member

Amanieu commented Apr 28, 2018

I found this method useful when I had an array of several BinaryHeaps from which I needed to pop one item:

while let Some(work_item) = work_lists.iter_mut().find_map(|h| h.pop()) {
    ...
}

Additionally, the order of the heaps in the array is significant, which this method handles perfectly (the first heap in the array has highest priority).

@shepmaster
Copy link
Member

I have also found it useful.

Before

LENGTHS.flat_map(|length| {
    seeds
        .par_iter()
        .find_any(|&&seed| test_password(length, seed))
        .map(|&seed| (length, seed))
}).next()

After

LENGTHS.find_map(|length| {
    seeds
        .par_iter()
        .find_any(|&&seed| test_password(length, seed))
        .map(|&seed| (length, seed))
})

@matklad
Copy link
Member

matklad commented Aug 9, 2018

I wonder what are the next steps here? Are we ready to propose this for stabilization? If we are, what's the process? :)

@shepmaster
Copy link
Member

what's the process? :)

Code mechanics wise: So you want to stabilize a feature?.

@KodrAus
Copy link
Contributor Author

KodrAus commented Aug 12, 2018

This is a handy little method for iterators, that @matklad nicely motivated in the original PR. Shall we stabilize?

@rfcbot fcp merge

Pessimistically cc'ing @rust-lang/libs in case rfcbot ignores me.

@alexcrichton
Copy link
Member

@rfcbot fcp merge

Let's try again!

@rfcbot
Copy link

rfcbot commented Aug 12, 2018

Team member @alexcrichton has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Aug 12, 2018
@rfcbot
Copy link

rfcbot commented Aug 14, 2018

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Aug 14, 2018
@leonardo-m
Copy link

leonardo-m commented Aug 15, 2018

The development strategy of this part of the standard library is broken. A better strategy should be: put the iterators you want in a external library (like itertools), and after a while of usage and testing and refinement move the top used iterators of the library/libraries into the std library. There are iterators in itertools that are far more used and far more useful than find_map, flatten and other things proposed to add or added to the std library.

@matklad
Copy link
Member

matklad commented Aug 15, 2018

Not a member of libs team, but I don't think there's mutual exclusion here. Moving most used methods from itertools to stdlib is an obviously good idea, and I think the only reason it's not done yet is that nobody has actually done the job of finding most useful methods, moving docs/tests/code to rust-lang and submitting PRs. If you could do this work, or maybe spearhead some "call for participation" style effort in this area, that would be absolutely awesome.

The only thing to keep in mind is that number of usages shouldn't be the only criterion. I sort of feel that "the API feels 100% correct" is also important. I love Itertools:::join and Itertools::group_by, and those APIs are probably more useful that find-map. However, the don't feel obviously right: join does not work with io::Write, and group_by has this non-trivial buffering behavior.

As for this method specifically, I've specified the Itertools route as an alternative in the original PR, together with arguments for why should it go stdlib directly, and a survey of real-world usages of find_map-like patterns.

@matklad
Copy link
Member

matklad commented Aug 15, 2018

optimistically send a stabilization PR: #53385

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Aug 24, 2018
@rfcbot
Copy link

rfcbot commented Aug 24, 2018

The final comment period, with a disposition to merge, as per the review above, is now complete.

bors added a commit that referenced this issue Aug 25, 2018
Stablize Iterator::find_map

Stabilization PR for #49602
@matklad
Copy link
Member

matklad commented Aug 25, 2018

closed by #53385 (comment)

@kennytm
Copy link
Member

kennytm commented Oct 27, 2018

This function has been stabilized in 1.30.0

@kennytm kennytm closed this as completed Oct 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-iterators Area: Iterators C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

9 participants