Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

required keyword arguments? #5111

Closed
StefanKarpinski opened this issue Dec 11, 2013 · 17 comments · Fixed by #25830
Closed

required keyword arguments? #5111

StefanKarpinski opened this issue Dec 11, 2013 · 17 comments · Fixed by #25830
Labels
keyword arguments f(x; keyword=arguments)
Milestone

Comments

@StefanKarpinski
Copy link
Member

Example use case:

copy!(; dst, src)

# usage:
copy!(src=A, dst=B)
copy!(dst=A, src=B)

Not actually suggesting changing copy! it's just a function where there isn't actually an obvious argument order, so using keywords might be a good idea.

@timholy
Copy link
Member

timholy commented Dec 11, 2013

The problem is that copy! is used in a lot of performance-critical scenarios.

@stevengj
Copy link
Member

You can implement required keywords now:

function foo(; bar=:required)
    if bar == :required
       throw(ArgumentError("bar keyword argument must be specified"))
    end
    ...
end

In the case of copy!, I'm concerned about the performance aspects as @timholy suggests.

@StefanKarpinski
Copy link
Member Author

Copy was just an example. I'm not proposing actually changing it.

@JeffBezanson
Copy link
Member

I think this is a spurious feature. And it clashes a bit with how keyword args are implemented --- they cannot actually be required, all you can do is throw an error if they aren't passed. So for example the method copy!() would appear to exist but it only throws an error.

@JeffBezanson
Copy link
Member

I believe @vtjnash pointed out that you can use a throw as a default value to get this effect, f(; req=error("req argument is required"). I think that's the best way to get this feature, requiring no internal changes.

@StefanKarpinski
Copy link
Member Author

It would be nice to do this automatically when no default is supplied for a keyword argument.

@vtjnash
Copy link
Member

vtjnash commented Nov 20, 2014

Agreed. And this is different from dispatching on the names, which as Jeff pointed out, would clash with dispatch. It would also be nice to make this a type, so that we can generate a minimal amount of code to throw the error.

@JeffBezanson
Copy link
Member

Ok, that all sounds good.

@stevengj
Copy link
Member

I don't know about making this a type, because I would want to be able to define the type of the keyword independently of whether it is required. e.g. f(; req::Int=requiredkeyword(:req)). But it would be a lot nicer to just do f(; req::Int) and have it automatically throw an error if not assigned.

@vtjnash
Copy link
Member

vtjnash commented Nov 21, 2014

@stevengj i mean that the error should have a type, not just a string, so that the cost of making the string is deferred until the user tries to print the exception

@stevengj
Copy link
Member

Ah yes, an UnassignedKeyword(kw::Symbol, T::Type) <: Exception type makes sense here.

@StefanKarpinski
Copy link
Member Author

We can add this feature at any point since it's currently a syntax error, but the justification for having this be an actual built-in feature even though one can easily simulate it by having a default expression that throws an error is just to make the error messaging uniform and avoid repeated boilerplate of writing that code by hand.

@yurivish
Copy link
Contributor

yurivish commented Nov 22, 2017

There's also a slight consistency benefit – being able to explain that keyword arguments are just like positional arguments except that they are passed by name.

I'm writing a short article on default function arguments and having to note that they're always required for keyword arguments stuck out to me as something that "just happens to be that way" rather than stemming from a deeper design choice, at least as far as I've been able to make out.

@JeffBezanson
Copy link
Member

In our case, the deeper design choice is that we don't dispatch on keyword arguments. In f(x, y) both arguments are only "required" in the sense that if you don't pass them, another method (or no method) will be selected.

As discussed above, we could implement required keyword arguments by converting function f(x; k) to function f(x; k = error("must pass k")) but there would still be a difference in that you'd get a must pass k error instead of a method error.

@Ismael-VC
Copy link
Contributor

How about function foo(x; k = throw(ArgumentError("must pass k")))?

@StefanKarpinski
Copy link
Member Author

I think that's fine: I never said this should be a no method error, it clearly shouldn't. It should either be an ArgumentError or something like that.

@zenna
Copy link

zenna commented Jan 31, 2018

In the meantime, I use a little macro

"""Convenience macro for requiring keyword arguments

``jldoctest
julia> f(x; @req(y), z = 3) = x + y + z
f (generic function with 2 methods)

julia> f(3)
ERROR: ArgumentError: y is required kwarg
Stacktrace:
 [1] f(::Int64) at ./REPL[29]:1

julia> f(3, y=2)
8
``
"""
macro req(kwd::Symbol)
  skwd = "$kwd is required kwarg"
  argerror = :(throw(ArgumentError($(esc(skwd)))))
  Expr(:kw, kwd, argerror)
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keyword arguments f(x; keyword=arguments)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants