This is very different from what I’m describing. I’m suggesting a collection of names to be used in the context of a specific domain, with the agreement that type piracy should be avoided. It should always be opt-in, never automatic.
I understand your case, and sometimes to avoid this kind of problem you simply extend a function that already exists (specially if it is from Base
), to avoid such name clashes. This happened to JuMP in a PR I did for JuMP. mlubin questioned the choice of extending Base.empty!
instead creating a JuMP.empty!
but I warned this would make us need to rewrite every empty!
on JuMP
code to qualify if it was the Base
one or the JuMP
one, and it would make it a headache for most JuMP users that simple do using JuMP
and create scripts with models (i.e., are not package mainteners). In the end, we extended Base.empty!
(I had also some more specific and convincing arguments).
Great example, thanks!
I probably would have voted for using the name reset!
or reset_model!
instead of extending Base.empty!
. The concept of resetting a JuMP model, although similar to the idea of emptying a collection, is a bit separate and should therefore have a different name.
This brings up a related issue that I don’t think has been discussed yet – discoverability. If you make a new name for your function in order to prevent name collisions, users need to be able to figure out what that is. Users familiar with something like empty!
or sample
will likely try to use it, get errors, then type ? empty!
which will not help them unless reset!
starts showing up in suggested similar functions. Maybe this is possible?
We do have a problem in Base where certain very generic words have been claimed for functions which are not very generic. For example:
help?> reset
reset(s)
Reset a stream s to a previously marked position, and remove the
mark. Return the previously marked position. Throw an error if the
stream is not marked.
help?> step
step(r)
Get the step size of an AbstractRange object.
step
should probably be called stepsize
. It’s more specific to what the function actually means. Additionally, stepsize
is clearly a noun rather than a verb (step
could be either), which indicates that you’re dealing with an accessor function.
Note that Gurobi.jl
and CPLEX.jl
did not implement Base.empty!
but instead MOI.empty!
that is the “fallback” of Base.empty!(::ModelLike)
and they also implemented MOI.is_empty
(basically MOI version of Base.isempty
but I think there is no Base.isempty(::ModelLike)
) both functions are used internally so the concept of a Model of some kind of container seems useful/adequate.
Yeah, I don’t mean to say that it is necessarily wrong, but it is a slippery slope. Just about any struct could be thought of as a container, especially if one is thinking at the more mechanical implementation level.
I think we’re in agreement that you shouldn’t be locked into using an API you don’t like. My only difference is on reusing the old function for your new API.
It’s true that
julia> rand!() = 1
rand! (generic function with 1 method)
julia> using Random
WARNING: using Random.rand! in module Main conflicts with an existing identifier.
can be frustrating to users, especially new ones. I think the experience can be improved significantly. In particular:
- “identifier” is compiler terminology, not user terminology
- What are the types and docs of the existing object and the new object
- Where are they each defined in this file and originally?
- What does it mean that they “conflict”? What is Julia doing about that? What does the name mean now?
- What can I do to avoid this problem?
There are problems for the end user.
- (All users) The help is hard to read.
?sample
grows really long and confusing. I’m disoriented and I don’t understand the semantics of the functions I’m using. - (All users) Nonetheless, I try to be a good developer and follow the rules, so I read the web docs of
AbstractMCMC
and try to callAbstractMCMC.sample
on myCScherrer.Chain
, and I pass itchain_type=
as documented, and it gives me a confusing error thatsample
has nochain_type
parameter. - (Advanced users) It breaks the automatic “it just works” magic of Julia’s interoperability: the ability to mix functions from one package with types from another. Suppose I actually want the semantics of
AbstractMCMC.sample
because I like how it handles errors (or logging, or missing data, or whatever), together withCScherrer.Chain
. Now I can’t just use that function even if they’re compatible, because it will automatically dispatch to the semantically wrong function and I won’t get the behavior I wanted unless I explicitly choosesample{AbstractMCMC.MCMCChain}(cscherrer_chain)
, sacrificing automatic interoperability (in the sense above), one of Julia’s best features.
Well, slippery slopes are often the fallacious type. I would say that a large amount of Julia types are just immutable
isbits
struct
s and those very clearly cannot be considered containers.
A JuMP Model:
- Often starts with no variables and constraints, unless they are explicitly given or it is loaded from somewhere.
- Has a good part of its interface dedicated to adding and removing variables/constraints from the Model object.
- The variables and constraints may be registered by name (if not anonymous)
model[:variable_or_constraint_name]
and if they are not, they can always at least be referred by an index that is given back by the object. - The own internal code cares about if there is something inside it (
MOI.is_empty
) and sometimesempty!
it to execute another action.
It can be a swan but at most distances it passes for a duck.
Yeah, I should have said mutable structs.
It’s not really spelled out in the manual, but I would say that Base.length
is the most essential method that should be defined for a collection. Since JuMP.Model
doesn’t have a length
method, it seems like a stretch to call it a collection.
Indexability does not necessarily imply that a type is a collection:
struct A
x
y
end
Base.getindex(a::A, sym) = getproperty(a, sym)
Anyhow, this is starting to diverge from the main topic of the thread…