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

Support kwargs in variable macro #2465

Merged
merged 1 commit into from
Feb 15, 2021
Merged

Support kwargs in variable macro #2465

merged 1 commit into from
Feb 15, 2021

Conversation

odow
Copy link
Member

@odow odow commented Feb 14, 2021

Closes #1884

@codecov
Copy link

codecov bot commented Feb 14, 2021

Codecov Report

Merging #2465 (4c0036b) into master (c432f66) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2465      +/-   ##
==========================================
+ Coverage   92.23%   92.25%   +0.01%     
==========================================
  Files          43       43              
  Lines        4652     4661       +9     
==========================================
+ Hits         4291     4300       +9     
  Misses        361      361              
Impacted Files Coverage Δ
src/macros.jl 92.30% <100.00%> (+0.15%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c432f66...4c0036b. Read the comment docs.

@blegat
Copy link
Member

blegat commented Feb 15, 2021

How did the keyword arguments work before this ? Looking at your code, it seems they are all merged into the first arguments and you need to flatten it at the end of the list.

@odow
Copy link
Member Author

odow commented Feb 15, 2021

How did the keyword arguments work before this?

They didn't. They just resulted in weird error messages:

julia> @variable(model; variable_type = :foo_bar)
ERROR: MethodError: no method matching _valid_model(::Expr; variable_type=:foo_bar)
Closest candidates are:
  _valid_model(::Any, ::Any) at /Users/oscar/.julia/dev/JuMP/src/macros.jl:39 got unsupported keyword argument "variable_type"
  _valid_model(::AbstractModel, ::Any) at /Users/oscar/.julia/dev/JuMP/src/macros.jl:38 got unsupported keyword argument "variable_type"
Stacktrace:
 [1] top-level scope at REPL[10]:1

@blegat
Copy link
Member

blegat commented Feb 15, 2021

Ah ok, what doesn't work is when you use a semicolon, I missed that

@odow odow merged commit 6bdf352 into master Feb 15, 2021
@odow odow deleted the od/kwarg_macro branch February 15, 2021 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

MethodError: Unexpected keyword argument to at-variable
2 participants