make Base.setindex() public

[aplavin]

setindex() is a widely used well-documented function, it should be marked public, right?

Not sure what's the right way to make a function with multiple methods public, please correct me if I'm wrong here!

[giordano]

I'm missing the point of this PR, as far as I can tell this is already public:

% julia +nightly -q
julia> Base.ispublic(Base, :setindex!)
true

[aplavin]

Note the difference:

julia> Base.ispublic(Base, :setindex!)
true

julia> Base.ispublic(Base, :setindex)
false

[aplavin]

Not sure what's the status here. Should I do some update of this PR or ...?

[nsajko]

If setindex is to be made public, it should be documented, i.e., its doc string should be included into the docs. Thus the "needs docs" label.

[aplavin]

I'm not familiar with documentation generation, could use some pointers on where to update things :)
Of course, assuming that this change is approved.

[nsajko]

If the function is to be made public, we'd need to decide if it's just for tuples, i.e, will adding methods for other collection types be allowed. Perhaps we could instead define something like setindex_tuple(t::Tuple, v, i) = setindex(t, v, i) and then make only setindex_tuple public.

Adding new methods would negatively affect abstract return type inference in the extreme case where the type of the input collection is unknown (inferred as Any), quite possibly requiring the ecosystem to add some type asserts for performance.

[ararslan]

(Removing my review request as @nsajko has got this handled 😉)

[aplavin]

Not sure what you exactly the issue with extending setindex and what exactly you suggest, @nsajko. Currently, setindex is defined (& documented) in Base for tuples and namedtuples. It's also used by lots of packages, with many more methods defined for setindex – eg, StaticArrays do that.

As the semantics of the function is pretty clear, and it's already extensively used in the wild (including new method definitions), what specific issues do you see with marking it public?

---

Perhaps we could instead define something like setindex_tuple(t::Tuple, v, i) = setindex(t, v, i) and then make only setindex_tuple public.

Tbh, I don't see a reason to do that. Defining separate functions for every type, instead of methods of the same functions, goes directly against Julia multiple dispatch design and practice.

[nsajko]

I'm referring to the "world splitting" optimization, see the Base.Experimental.@max_methods doc string. I agree that Base.setindex should probably be available for all collection types. But, it's good to consider the possible downsides upfront, seeing as, in a fresh REPL session, Base.setindex has just three methods - below the max_methods default.

EDIT: I only mentioned this issue for completeness. IMO it's secondary to proper documentation.

[nsajko]

May I ask, Is there a logic to this specific location, between hasproperty and getfield? Mostly just curious, I don't really have a better suggestion.

[nsajko]

Actually, perhaps the doc string would fit better into the Collections and Data Structures page? Under Indexable Collections I guess.

[aplavin]

Properties/fields and indices are kinda close conceptually, and they are exactly the same for tuples and namedtuples (the types setindex is defined for). So it seems to make sense to put it here.
But any location works for me.

[nsajko]

No, it can't go under "Indexable collections" as-is, because of the "fully implemented by" list in that section. The list includes types which don't implement setindex...

[nsajko]

FWIW, given that setindex is indeed already used in many packages for some reason, and its interface seems OK, I now agree that it should probably be made public.

[aplavin]

I'm referring to the "world splitting" optimization, <...>

Still not sure how these considerations are relevant to marking setindex as public.
I don't propose adding any more methods in Base here, although, I would like that as a completely separate independent PR (#43155, #54508, #43338, #33495). Marking it as public should just reflect the practical status quo: this function is widely used & overloaded.

Btw, a lot of high-performance code uses StaticArrays, and with them loaded one has at least 6 setindex methods.

[aplavin]

bump...

[aplavin]

bump :)

[aplavin]

bump... anything else I should do here?

[aplavin]

and another bump

[aplavin]

bump, anyone?..

[aplavin]

should be a tiny change, any issues with it?..