-
-
Notifications
You must be signed in to change notification settings - Fork 117
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 #586
Conversation
Restarting CI. |
I suggest that you add a
to make it look the same as on Julia 0.7. |
test/runtests.jl
Outdated
@test func2(3) == 3 # using the function works | ||
@compat func3(;y) = y | ||
@test func3(y=2) == 2 # using the function works | ||
try |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of try-catch you can use @test_throws
, for example
julia> f(;y) = y
f (generic function with 1 method)
julia> @test_throws UndefKeywordError f()
Test Passed
Thrown: UndefKeywordError
thanks @fredrikekre , I will add that |
The errors seem to be due to the if !isdefined(:UndefKeywordError)
struct UndefKeywordError <: Exception
kw
end
Base.showerror(io::IO, e::UndefKeywordError) = print(io, "keyword argument $(e.kw) not assigned")
export UndefKeywordError
end ? |
Yes, you can use |
Thanks, seems to work now! |
src/compatmacro.jl
Outdated
struct UndefKeywordError <: Exception | ||
kw | ||
end | ||
Base.showerror(io::IO, e::UndefKeywordError) = print(io, "keyword argument $(e.kw) not assigned") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, this should also print the exception type, so
print(io, "UndefKeywordError: keyword argument $(e.kw) not assigned")
as it does on Julia v0.7:
julia> f(;y) = y
f (generic function with 1 method)
julia> f()
ERROR: UndefKeywordError: keyword argument y not assigned
otherwise LGTM
True, corrected that. |
Needs a README item. |
@stevengj: absolutely. I added a description in the README, is it okai like this? |
README.md
Outdated
@@ -79,6 +79,8 @@ Currently, the `@compat` macro supports the following syntaxes: | |||
`foo(::CartesianRange{CartesianIndex{N}})` ([#20974]). Note that | |||
`CartesianRange` now has two type parameters, so using them as | |||
fields in other `struct`s requires manual intervention. | |||
|
|||
* required keyword arguments: e.g. `@compat foo(;x, y)` makes `x` and `y` required keyword arguments. When calling `foo` an error is thrown if `x` and `y` are not explictly provided |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some fixes:
* Required keyword arguments ([#25830]). For example, `@compat foo(; x, y)` makes `x` and `y` required keyword arguments: when calling `foo`, an error is thrown if `x` or `y` is not explicitly provided.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(The link to JuliaLang/julia#25830 is added if you run julia /path/to/julia/doc/NEWS-update.jl README.jl
to run https://github.com/JuliaLang/julia/blob/master/doc/NEWS-update.jl)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just pushed the update README.md.
@stevengj I responded to your comment and updated the README.md a few days ago. Do you agree with the result? |
Thanks! |
function symbol2kw(sym::Symbol) | ||
Expr(:kw, sym, Expr(:call, throw, UndefKeywordError(sym))) | ||
end | ||
symbol2kw(arg) = arg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't handle the case where the argument has a type. It should probably be:
arg2kw(arg, sym) = Expr(:kw, arg, Expr(:call, throw, UndefKeywordError(sym)))
symbol2kw(arg::Symbol) = arg2kw(arg, arg)
symbol2kw(arg::Expr) = isexpr(arg, :(::)) ? arg2kw(arg, arg.args[1]) : arg
symbol2kw(arg) = arg
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that seems reasonable, should I update the PR? Can I even or should this be a new PR? |
* Bump required Julia version to 1.0 * Remove compatibility support code for: * `at-__MODULE__` (from #363) * `devnull`, `stdin`, `stdout`, and `stderr` from #499 * `at-nospecialize` (from #385 and #409) * `isabstracttype` and `isconcretetype` (from #477) * `invokelatest` from #424 * array-like access to `Cmd` from #379 * `Val(n)` and `ntuple`/`reshape` with `Val` from #381 and #399 * `logdet(::Any)` fallback from #382 * `chol(::UniformScaling)` from #382 * `pushfirst!`, `popfirst!` from #444 * `fieldcount` from #386 * `read(obj, ::Type{String})` from #385 and #580 * `InexactError`, `DomainError`, and `OverflowError` constructors from #393 * `corrected` kw arg to `cov` from #401 * `adjoint` from #401 * `partialsort` from #401 * `pairs` from #428 * `AbstractRange` from #400 * `rtoldefault` from #401 * `Dates.Period` rounding from #462 * `IterativeEigensolvers` from #435 * `occursin` from #520 * `Char` concatenation from #406 * `BitSet` from #407 * `diagm` and `spdiagm` with pairs from #408 * `Array` c'tors from `UniformScaling` from #412 and #438 * `IOContext` ctor taking pairs from #427 * `undef` from #417 and #514 * `get` on `ENV` from #430 * `ComplexF...` from #431 * `tr` from #614 * `textwidth` from #644 * `isnumeric` from #543 * `AbstractDict` from #435 * `axes` #435 and #442 * `Nothing` and `Cvoid` from #435 * `Compat.SuiteSparse` from #435 * `invpermute!` from #445 * `replace` with a pair from #445 * `copyto!` from #448 * `contains` from #452 * `CartesianIndices` and `LinearIndices` from #446, #455, and #524 * `findall` from #466 (and #467). * `argmin` and `argmax` from #470, #472, and #622 * `parentmodule` from #461 * `codeunits` from #474 * `nameof` from #471 * `GC` from #477 * `AbstractDisplay` from #482 * `bytesavailable` from #483 * `firstindex` and `lastindex` from #480 and #494 * `printstyled` from #481 * `hasmethod` from #486 * `objectid` from #486 * `Compat.find*` from #484 and #513 * `repr` and `showable` from #497 * `Compat.names` from #493 and #505 * `Compat.round` and friends #500, #530, and #537 * `IOBuffer` from #501 and #504 * `range` with kw args and `LinRange` from #511 * `cp` and `mv` from #512 * `indexin` from #515 * `isuppercase` and friends from #516 * `dims` and `init` kwargs from #518, #528, #590, #592, and #613 * `selectdim` from #522 and #531 * `repeat` from #625 * `fetch(::Task)` from #549 * `isletter` from #542 * `isbitstype` from #560 * `at-cfunction` from #553 and #566 * `codeunit` and `thisind` and friends from #573 * `something` from #562 * `permutedims` from #582 * `atan` from #574 * `split` and `rsplit` from #572 * `mapslices` from #588 * `floatmin` and `floatmax` from #607 * `dropdims` from #618 * required keyword arguments from #586 * `CartesianRange` in `at-compat` from #377 * `finalizer` from #416 * `readline`, `eachline`, and `readuntil` from #477, #541, and #575 * curried `isequal`, `==`, and `in` from #517 * `Some` from #435 and #563 * `at-warn` and friends from #458 * Remove old deprecations * Deprecate: * `Compat.Sockets` from #545 and #594 * `TypeUtils` from #304 * `macros_have_sourceloc` from #355 * `Compat.Sys` from #380, #433, and #552 * `Compat.MathConstants` from #401 * `Compat.Test`, `Compat.SharedArrays`, `Compat.Mmap`, and `Compat.DelimitedFiles` from #404 * `Compat.Dates` from #413 * `Compat.Libdl` from #465 (and #467) * `AbstractDateTime` from #443 * `Compat.Printf` from #435 * `Compat.LinearAlgebra` from #463 * `Compat.SparseArrays` from #459 * `Compat.Random` from #460, #601, and #647 * `Compat.Markdown` from #492 * `Compat.REPL` from #469 * `Compat.Serialization` from #473 * `Compat.Statistics` from #583 * `Fix2` from #517 * `Compat.Base64` from #418 * `Compat.Unicode` from #432 and #507 * `notnothing` from #435 and #563 * `Compat.IteratorSize` and `Compat.IteratorEltype` from #451 * `enable_debug(::Bool)` from #458 * `Compat.Distributed` from #477 * `Compat.Pkg` from #485 * `Compat.InteractiveUtils` from #485 * `Compat.LibGit2` from #487 * `Compat.UUIDs` from #490 * `Compat.qr` from #534 * `Compat.rmul!` from #546 * `Compat.norm` abd friends from #577 * Remove obsolete README entry, missed in #385 * Remove obsolete tests (e.g. missed in #372) * Remove obsolete `VERSION` conditionals and some minor clean-up
Following up on the comment by @Keno in this PR JuliaLang/METADATA.jl#15489 I added the functionality to understand required keyword arguments to
@compat
. Basically that meansis equivalent to
As this is my first submission to a JuliaLang rep, could you check that the style etc. matches what you would expect?