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

🚸 OSQPOptimizer -> OSQP.Optimizer #34

Merged
merged 3 commits into from Oct 15, 2018
Merged

Conversation

blegat
Copy link
Collaborator

@blegat blegat commented Oct 14, 2018

No description provided.

@codecov-io
Copy link

codecov-io commented Oct 14, 2018

Codecov Report

Merging #34 into master will increase coverage by 0.02%.
The diff coverage is 92.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #34      +/-   ##
==========================================
+ Coverage   86.28%   86.31%   +0.02%     
==========================================
  Files           6        6              
  Lines         970      972       +2     
==========================================
+ Hits          837      839       +2     
  Misses        133      133
Impacted Files Coverage Δ
src/OSQP.jl 83.33% <ø> (ø) ⬆️
src/MOIWrapper.jl 90.9% <92.85%> (+0.04%) ⬆️

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 53140cb...49d95c9. Read the comment docs.

export OSQPOptimizer
function OSQPOptimizer()
Base.depwarn("OSQPOptimizer() is deprecated, use OSQP.Optimizer() instead.",
:OSQPOptimizer)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this function return Optimizer() to avoid breaking existing code?

Copy link
Collaborator

@tkoolen tkoolen Oct 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does actually:

julia> using OSQP.MathOptInterfaceOSQP
[ Info: Precompiling OSQP [ab2f91bb-94b4-55e3-9ba0-7f65df51de79]

julia> optimizer = OSQPOptimizer()
┌ Warning: OSQPOptimizer() is deprecated, use OSQP.Optimizer() instead.
│   caller = top-level scope at none:0
└ @ Core none:0
Optimizer(OSQP.Model(Ptr{OSQP.Workspace} @0x0000000000000000, Float64[], Float64[]), false, OSQP.Results(Float64[], Float64[], OSQP.Info(140496452123152, #undef, 569348, 3, 1.5e-323, 0.0, 0.0, 0.0, 6.94145239783165e-310, 6.94144482996333e-310, 6.9414448298977e-310, 0, 0.0), Float64[], Float64[]), true, Dict{Symbol,Any}(), MinSense::OptimizationSense = 0, 0.0, Float64[], OSQP.MathOptInterfaceOSQP.ModificationCaches.ProblemModificationCache{Float64}(#undef, #undef, #undef, #undef, #undef), OSQP.MathOptInterfaceOSQP.ModificationCaches.WarmStartCache{Float64}(#undef, #undef), Dict{Int64,UnitRange{Int64}}())

Note that this is actually exactly equivalent to

Base.@deprecate OSQPOptimizer() OSQP.Optimizer()

No need to change this in this PR, just FYI.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I hadn't noticed b52ad84.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot about this macro, @tkoolen thanks for the tip :)

Copy link
Collaborator

@tkoolen tkoolen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thanks for separating this commit out, @blegat!

@tkoolen tkoolen merged commit 1748fc0 into master Oct 15, 2018
@tkoolen tkoolen deleted the bl/nonexportedoptimizer branch October 15, 2018 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants