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

Proposal to use map!(f,A) for in place modification of collections #31677

Open
ndinsmore opened this issue Apr 10, 2019 · 6 comments
Open

Proposal to use map!(f,A) for in place modification of collections #31677

ndinsmore opened this issue Apr 10, 2019 · 6 comments
Labels
domain:collections Data structures holding multiple items, e.g. sets

Comments

@ndinsmore
Copy link
Contributor

I have been working through some of the JuliaCollection packages working to make sure that map!(f,values(dict)) has non-naive implementations. As the semantic was introduced in #31223.

Which got me thinking that potentially map! should be implemented on more of the non AbstractDict collection\container types as it could be useful.

For those types that just store values without keys, it would make sense that map!(f, collection) is the syntax. But apparently, that exact syntax was depreciated in #19721. There is an open PR to add it back in for AbstractArray, #3007.

There are some related open issues like:
#12277

It seems to me that map!(f, A) should clearly be equivalent to map!(f,A,A) so can the ecosystem use that?
Finally, if ecosystem packages implement map!(f,A) for in place value modification, is there a way to make sure that when map!(f,A,A) is used it can get a fall back to map!(f,A)?

@ndinsmore ndinsmore changed the title Approval of map!(f,A) for in place modification of collections Proposal to use map!(f,A) for in place modification of collections Apr 10, 2019
@Jutho
Copy link
Contributor

Jutho commented Apr 16, 2019

I think that #12277 exactly argued why map!(f,A) behaving as map!(f,A,A) is confusing, given that broadcast!(f,A) does something else (namely call f without arguments). As such, #12277 was the discussion that lead to #19721, which I don't think stands any chance of being reverted.

@rfourquet rfourquet added the domain:collections Data structures holding multiple items, e.g. sets label Jun 25, 2020
@StefanKarpinski
Copy link
Sponsor Member

FWIW, I think we should get rid of all the "nullary" methods of map et al., so that map(f) means (args...) -> map(f, args...) and make map!(f, x) mean map!(f, x, x). I really doubt that anyone is actually depending on the nullary behavior.

@StefanKarpinski StefanKarpinski added the status:triage This should be discussed on a triage call label Jun 25, 2020
@MasonProtter
Copy link
Contributor

MasonProtter commented Jun 25, 2020

It's also pretty easy to get the "nullary" behaviour if you want it like this: map(_ -> f(), x) or map!(_ -> f(), x) for inplace.

@tkf
Copy link
Member

tkf commented Jun 25, 2020

FYI #35293 is an RFC for gettring rid of map(f) and foreach(f).

@StefanKarpinski
Copy link
Sponsor Member

I think we should do both that and this. We could try it and if it breaks any package, reconsider, but I bet it won't.

@StefanKarpinski StefanKarpinski added the kind:minor change Marginal behavior change acceptable for a minor release label Jun 29, 2020
@LilithHafner LilithHafner added needs nanosoldier run This PR should have benchmarks run on it and removed status:triage This should be discussed on a triage call labels Oct 27, 2023
@LilithHafner
Copy link
Member

Triage says we should do this; there's really no other sensible interpretation of map!(f, x).

@LilithHafner LilithHafner removed needs nanosoldier run This PR should have benchmarks run on it kind:minor change Marginal behavior change acceptable for a minor release labels Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:collections Data structures holding multiple items, e.g. sets
Projects
None yet
Development

No branches or pull requests

7 participants