Closed aplavin closed 2 years ago
OK, interesting. Thanks @aplavin.
This is possibly fine here but elsewhere the API is a bit strange and you need to disambiguate which terms are collections and which are functions. I don't really like using Base.Callable
in these cases but its a necessary evil and hard to work around given how similar we like to keep the API to Base
, etc, although at some point we should just go ahead and make breaking changes and split functions up a bit.
(Another thing which would be nice with map
etc is if they could be curried, but zero-collection map
is defined in Base as a kind of 0-dimensional style of generalization, so I'd like to consider also how this fits together).
PS I have created an issue in Accessors.jl to see if the lenses etc could subtype Function
.
As I understand, the major ambiguity is in the group
function - its first argument can either be a callable or an array. This PR is focused on mapview
alone, so there shouldn't be any such issues.
Actually, I encountered this incompatibility (Accessors.jl lenses vs mapview) when trying to make mapview
result support setindex!
. It turned out to be surprisingly easy: the only thing needed in addition to relaxed type constraints (remove Base.Callable
- this PR) is this method:
Base.setindex!(ma::SplitApplyCombine.MappedArray, value, ix) = ma.parent[ix] = Accessors.set(ma.parent[ix], ma.f, value)
Then assigning to mapview
elements just works:
Of course, this method cannot really be put into either of these packages because they don't depend on each other, so currently I use it in a type-piratic way.
There is no ambiguity in the first mapview() argument anyway. This makes it possible to use any callable (not covered by
Base.Callable
) asf
inmapview
- for example,@optic(...)
from Accessors.jl.