Document map behavior on sparse data structures

[ParadaCarleton]

This pull request clarifies that map's behavior assumes purity when working with sparse data structures, and that such behavior is not considered a bug, as discussed in this thread. If the core devs clarify that this behavior is intentional and map requires purity, I can also add an apply function which iterates through a given collection and applies a function element-by-element.

[ParadaCarleton]

Bump -- @JeffBezanson @StefanKarpinski am I correct in assuming that map's purity assumption is intended and should be documented?

[StefanKarpinski]

I don't think either of us are the ones to address that but yes, the assumption that the mapper is consistent in mapping values to zero is intended and should be documented.

[ParadaCarleton]

I don't think either of us are the ones to address that but yes, the assumption that the mapper is consistent in mapping values to zero is intended and should be documented.

Perfect; do you know who I should mention (if I should mention anyone)?

[StefanKarpinski]

I believe @andreasnoack may be able to help at least by recommending someone who could help more.

[bkamins]

it will be called once on default 0 but if you have duplicates that are non-default zeros it will be called multiple times.

[ParadaCarleton]

it will be called once on default 0 but if you have duplicates that are non-default zeros it will be called multiple times.

I've edited to say that it "May" call it only once per duplicate element, and clarify that calling map with an impure function is undefined behavior.

[bkamins]

The performance rationale behind the proposal of @ParadaCarleton is clear.

However, when we accept it we should update docstrings of map!, mapreduce, mapslices, and Iterators.map accordingly.

In general the issue is that users might have assumed that it is OK to use these functions with non-pure argument. E.g. Iterators.map contract says:

This is another syntax for writing (f(args...) for args in zip(iterators...)).

and generator equivalent for should not assume purity of f (at least in my opinion).

[ParadaCarleton]

The performance rationale behind the proposal of @ParadaCarleton is clear.
However, when we accept it we should update docstrings of map!, mapreduce, mapslices, and Iterators.map accordingly.
In general the issue is that users might have assumed that it is OK to use these functions with non-pure argument. E.g. Iterators.map contract says:

This is another syntax for writing (f(args...) for args in zip(iterators...)).
and generator equivalent for should not assume purity of f (at least in my opinion).

While I think I'd agree we should have some alternative to map that doesn't assume purity, I'd say Iterators.map is already assuming purity implicitly because of its laziness. While in theory it's possible to have a lazily-called impure function, laziness+statefulness is a recipe for disaster.

I agree that we should avoid making any changes to behavior for now to avoid breaking old code, though.

[bkamins]

While in theory it's possible to have a lazily-called impure function, laziness+statefulness is a recipe for disaster.

100% agreed. The only problem is exactly what you have commented on later - old code might be affected (fortunately we are not changing implementation of anything - only the contract). As you know in DataFrames.jl we identified this issue because users reported to us problems when they used not-pure functions with map.