Unique names don't require underscores#161
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #161 +/- ##
==========================================
+ Coverage 94.73% 94.76% +0.02%
==========================================
Files 13 13
Lines 703 707 +4
==========================================
+ Hits 666 670 +4
Misses 37 37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| func = get_function(primal_model, ci) | ||
| set = get_set(primal_model, ci) | ||
| for (i, vi) in enumerate(vis) | ||
| unique_var = length(vis) == 1 |
There was a problem hiding this comment.
This can be moved one line above outside the for loop
| i::Int, | ||
| ci_name::String, | ||
| prefix::String, | ||
| unique_var::Bool = false, |
There was a problem hiding this comment.
We don't need a default value if we set this value everywhere we use this function. Otherwise that just adds a chance to forget to set this and have a silent bug
There was a problem hiding this comment.
was to avoid breaking things but fine with no default
There was a problem hiding this comment.
I think it's fine, we can always tag v0.6, we haven't hit v1 yet
| i::Int, | ||
| ci_name::String, | ||
| prefix::String, | ||
| unique_var::Bool, |
There was a problem hiding this comment.
How about ; ensure_unique::Bool = true,.
If this is a public method, then we don't need to break things. Appending a false isn't very informative for the caller.
There was a problem hiding this comment.
sounds good. It's not super important since this is internal anyway
|
replaced by #184 |
4 participants
All models end up marking each variable group with
_$i, even though the variables themselves are often in containers.This typically ends up with a double indexing scheme in JuMP:
This PR adapts it to avoid the
_iif the variable name is unique