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

Delete map(f, ::AbstractString) special case #57071

Open
LilithHafner opened this issue Jan 16, 2025 · 8 comments
Open

Delete map(f, ::AbstractString) special case #57071

LilithHafner opened this issue Jan 16, 2025 · 8 comments
Labels
breaking This change will break code iteration Involves iteration or the iteration protocol strings "Strings!"

Comments

@LilithHafner
Copy link
Member

LilithHafner commented Jan 16, 2025

Do we actually want this?

julia> map(x -> x+1, "abc")
"bcd"

julia> map((x,y) -> x+1, "abc", [1,2,3])
3-element Vector{Char}:
 'b': ASCII/Unicode U+0062 (category Ll: Letter, lowercase)
 'c': ASCII/Unicode U+0063 (category Ll: Letter, lowercase)
 'd': ASCII/Unicode U+0064 (category Ll: Letter, lowercase)

Is it worth the break from the typical semantics of map where the function always returns an AbstractArray?

Can we rename this method to stringmap?

@LilithHafner LilithHafner added breaking This change will break code iteration Involves iteration or the iteration protocol strings "Strings!" labels Jan 16, 2025
@LilithHafner
Copy link
Member Author

The tuple case is also a special case, though imo a much more reasonable special case.

@LilithHafner
Copy link
Member Author

First step: add stringmap & deprecate that map method.

@jakobnissen
Copy link
Contributor

map is also defined for Number.
I don't exactly understand what map is supposed to do. As far as I can tell, there are three useful behaviours, and it currently does neither of these:

  1. A purely lazy map - which is the different Iterators.map
  2. A function that always returns the same container, e.g. a Vector or whatever.
  3. A function that preserves the container type which allows it to be generic over the container

Right now we're in this weird halfway point where it sometimes returns a Vector, sometimes returns a container of the same type (Number, String, tuple, some arrays), and sometimes errors (sets and dicts).

@jariji
Copy link
Contributor

jariji commented Jan 16, 2025

As I said in #51703, I think it's necessary to decide if map is required to return a certain concrete/abstract type, or if instead it has certain properties. Option 1:

  • map returns AbstractArray: It's okay if we want to say map has to return AbstractArray.

But abstract types are somewhat restrictive. It would be more flexible if we adopted option 2:

  • map has properties: There's a list of properties that a valid implementation of map must have: always returns an indexable collection preserving the indices and size of its argument when they exist.

If map(f, ::Tuple)::Tuple is okay, that can be made principled by a specification. If that specification would apply to Tuple, and would apply to String, then can be correctly implemented for both. (It probably wouldn't work on Set because that doesn't preserve length, so map(f, ::Set)::Set wouldn't be implemented.)

@vtjnash
Copy link
Member

vtjnash commented Jan 16, 2025

map on a String (currently) does not preserve indices, since it is indexed by byte offsets but iterates by characters, so it is a fairly specific special case to whatever definition we use

@MasonProtter
Copy link
Contributor

Example of what Jameson is talking about:

julia> let str = "αγβ"
           str2 = map(_ -> 'c', str)
           collect(eachindex(str)), collect(eachindex(str2))
       end
([1, 3, 5], [1, 2, 3])

@jariji
Copy link
Contributor

jariji commented Jan 16, 2025

I'm sold on removal then.

@topolarity
Copy link
Member

Should we document that map impls should preserve both the keys(...) and iteration order of its argument (ignoring the multi-dimensional case), and does not guarantee anything about the result type otherwise?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking This change will break code iteration Involves iteration or the iteration protocol strings "Strings!"
Projects
None yet
Development

No branches or pull requests

6 participants