Skip to content

Add filter(f, itr) = collect(Iterators.filter(f, itr)) #37195

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

Closed
wants to merge 1 commit into from

Conversation

mcabbott
Copy link
Contributor

Closes #37146

@bramtayl
Copy link
Contributor

Unrelated but maybe we could change the map method to

map(f, iters...) = collect(Iterators.map(f, iters...))

Another thing worth thinking about eventually is whether we need auto-collecting versions of iterators in the first place?

@mcabbott
Copy link
Contributor Author

mcabbott commented Sep 4, 2020

Aren't those identical?

julia> Iterators.map(sqrt, 1:10) === Base.Generator(sqrt, 1:10)
true

@bramtayl
Copy link
Contributor

bramtayl commented Sep 4, 2020

Yes, but I think Iterators.map is intended to be a public and overloadable interface, whereas Base.Generator is an internal, I think.

@mcabbott
Copy link
Contributor Author

Bump?

cc. @JeffBezanson since he said 👍 earlier...

@StefanKarpinski StefanKarpinski added the triage This should be discussed on a triage call label Nov 16, 2020
@Gyoshi
Copy link

Gyoshi commented Dec 29, 2020

map has been explicitly deprecated for Sets and Dicts (#26980):

map(f, ::AbstractDict) = error("map is not defined on dictionaries")
map(f, ::AbstractSet) = error("map is not defined on sets")

Maybe this should also hold for filter?

@JeffBezanson
Copy link
Member

No; map and filter are different in important ways. map changes values, so it's less clear what it should do for structures that impose invariants on the values they hold (e.g. that they are unique). filter just removes some elements.

I'm not sure we should add this. I think the fallback map definition is kind of a misfeature. We shouldn't encourage collecting all elements into an array until it's really needed. Anyway there's no conclusion from triage posted here, so let's wait for that.

@JeffBezanson
Copy link
Member

Nobody on triage this week seems to feel strongly positive about this.

@mcabbott mcabbott closed this May 7, 2022
@LilithHafner LilithHafner removed the triage This should be discussed on a triage call label Mar 2, 2023
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 this pull request may close these issues.

filter doesn't work on generators
6 participants