Use Base.Iterators.flatten instead of Iterators.chain #50
Conversation
| @@ -1,7 +1,7 @@ | |||
| using Combinatorics | |||
| using Base.Test | |||
|
|
|||
| @test [combinations([])...] == [] | |||
| @test_skip [combinations([])...] == [] | |||
There was a problem hiding this comment.
Does this really make sense? I.e. should [combinations([])...] == [] actually work instead of just throwing an Argument error?
hmm I see that
julia> combinations([],0) |>collect
0-element Array{Array{Any,1},1}
There was a problem hiding this comment.
yeah I am not sure this really should work. There aren't no combinations of nothing, it's just undefined? I guess what really matters is whether completeness here really matters. It doesn't seem useful, and I'd rather see an error here.
There was a problem hiding this comment.
The way it was implemented was consistently with the fact that
julia> binomial(0,0)
1
julia> length(combinations([], 0))
1
Codecov Report
@@ Coverage Diff @@
## master #50 +/- ##
=======================================
Coverage 96.96% 96.96%
=======================================
Files 6 6
Lines 594 594
=======================================
Hits 576 576
Misses 18 18
Continue to review full report at Codecov.
|
| @@ -1,7 +1,7 @@ | |||
| using Combinatorics | |||
| using Base.Test | |||
|
|
|||
| @test [combinations([])...] == [] | |||
| @test_skip [combinations([])...] == [] | |||
There was a problem hiding this comment.
yeah I am not sure this really should work. There aren't no combinations of nothing, it's just undefined? I guess what really matters is whether completeness here really matters. It doesn't seem useful, and I'd rather see an error here.
|
LGTM. But probably need other eyes since I am not a contributor here. This change avoids a temporary and should be strictly better anyways. |
|
@jiahao is it okay if I ping you here? These Iterators failures that the current setup has is very breaking. DiffEq v0.6 is fixed if this goes through (along with Roots.jl, though Bio.jl will still be broken since it has a direct dep to iterators). I would be willing to do the tagging after this if that's okay? |
|
In a strictly mathematical sense, the collection of all subsets should include the empty set. In this way, the argument will always have at least one member. Though, it may not be practical. |
No description provided.