Let variables take sets as well as vectors as input
#12
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #12 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 7 7
Lines 184 184
=========================================
Hits 184 184 ☔ View full report in Codecov by Sentry. |
|
/format |
|
✔️ Auto-formatting triggered by this comment succeeded, commited as 7259a63 |
src/constraint_tree.jl
Outdated
| @@ -191,7 +191,7 @@ interval bound for all variables, it is impossible to use a tuple (since its | |||
| length is 2); in such case use `bound = Ref((minimum, maximum))`, which has the | |||
| correct length. | |||
| """ | |||
| function variables(; keys::Vector{Symbol}, bounds = nothing) | |||
| function variables(; keys::Union{Vector{Symbol},Set{Symbol}}, bounds = nothing) | |||
There was a problem hiding this comment.
Do we really want a Set here? (it makes the order of variables in the created ConstraintTree basically unpredictable, which might be a tripwire in case people rely on the numbering and reproducibility e.g. in Distributed cluster, like we do every here and there for simplicity....)
Might be useful to just have AbstractVector here or so
|
For reproducibility I am closing this PR. (slack convo) |
|
actually we can extend a tiny bit |
Mostly to avoid re-allocating memory in other packages