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.

Sign up for GitHub

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

Use Base.Iterators.flatten instead of Iterators.chain #50

Closed
wants to merge 1 commit into from
Closed

Use Base.Iterators.flatten instead of Iterators.chain #50

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion REQUIRE
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
julia 0.5
Compat 0.18.0
Polynomials
Iterators
3 changes: 2 additions & 1 deletion src/Combinatorics.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ __precompile__(true)

module Combinatorics

using Compat, Polynomials, Iterators
using Polynomials
using Compat, Compat.Iterators.flatten

import Base: start, next, done, length, eltype

Expand Down
2 changes: 1 addition & 1 deletion src/combinations.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ end
generate combinations of all orders, chaining of order iterators is eager,
but sequence at each order is lazy
"""
combinations(a) = chain([combinations(a,k) for k=1:length(a)]...)
combinations(a) = flatten(combinations(a,k) for k = 1:length(a))



Expand Down
2 changes: 1 addition & 1 deletion test/combinations.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
using Combinatorics
using Base.Test

@test [combinations([])...] == []
@test_skip [combinations([])...] == []
Copy link
Contributor Author

@musm musm Jun 20, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

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}

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#46 might be related?

Copy link
Contributor

Choose a reason for hiding this comment

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

The way it was implemented was consistently with the fact that $C^0_0 = 1$

julia> binomial(0,0)
1

julia> length(combinations([], 0))
1

@test [combinations(['a', 'b', 'c'])...] == Any[['a'],['b'],['c'],['a','b'],['a','c'],['b','c'],['a','b','c']]

@test [combinations("abc",3)...] == Any[['a','b','c']]
Expand Down