trixi-framework
Use mandatory keyword arguments instead of positional arguments?
In most example files, we have
viscosity = ArtificialViscosityMonaghan(0.02, 0.0)
If you're not familiar with the API, you'd have to check the Julia help to see that the first positional argument is alpha and the second is beta. In many places, we do this instead:
alpha = 0.02
viscosity = ArtificialViscosityMonaghan(alpha, 0.0)
This makes it clear that the first positional argument is alpha, and it allows us to overwrite this with trixi_include. However, doing this everywhere will create quite a bit of extra code.
We could also use mandatory keyword arguments, so that the line above becomes
viscosity = ArtificialViscosityMonaghan(alpha=0.02, beta=0.0)
Here, we can natively use trixi_include without further modification.
The Trixi.jl API is slowly moving away from positional arguments for exactly this reason. But they often allow both options: https://github.com/trixi-framework/Trixi.jl/blob/276dc3ca22a4c72b81aa6659e57ff1382dd1ae0c/src/solvers/dgsem/dgsem.jl#L54-L69
Do we want to
- keep things like they are?
- have keyword arguments instead of positional arguments?
- allow both?
I don't like option 3, as it makes the API more complicating by adding more options to do the same thing.
Of course, when writing new code instead of modifying an existing example file, one has to first figure out the names for the keyword arguments (e.g., is it r or radius?), but with positional arguments, one has to figure out the order, so that doesn't make any difference to me.
In some cases, it makes sense to have kwargs. But having a hard rule to only allow kwargs in the API might be too restrictive, and in some cases, it makes the example files even more unreadable, IMO.
I can live with 1. and add kwargs where it makes sense (like so in ArtificialViscosityMonaghan). Also, we then have to think about default values, right?
I'm not talking about a hard rule or only using keyword arguments. Just for things like viscosity, state equations, setups maybe.
Also, we then have to think about default values, right?
No, you can just have kwargs without default values - it just makes them mandatory kwargs then. Usually it is much harder to use this in a wrong way than using only positional args. And if you insist on having the values defined elsewhere, you can also just use kwarg expansion, e.g.,
# Define kwarg-based ctor
function ArtificialViscosityMonaghan(alpha, beta) # (1)
...
end
ArtificialViscosityMonaghan(; alpha, beta) = ArtificialViscosityMonaghan(alpha, beta) # (2)
# Get values
alpha = get_alpha()
beta = get_beta()
# Construct in any of the following ways
ArtificialViscosityMonaghan(alpha, beta) # calls (1)
ArtificialViscosityMonaghan(alpha=alpha, beta=beta) # calls (2)
ArtificialViscosityMonaghan(; alpha, beta) # also calls (2)
# Compare wrong order
ArtificialViscosityMonaghan(beta, alpha) # calls (1) but with wrong order
ArtificialViscosityMonaghan(; beta, alpha) # calls (2) but all arguments are still correctly assigned!
