Skip to content

Conversation

@Datseris
Copy link

@Datseris Datseris commented Aug 9, 2018

I started to do this but I need help. I don't know what I am doing wrong, and haven't really understood what Combinatorics actually does. For now whenever I do [combinations(['a', 'b', 'c'])...] I get:

10-element Array{Array{Char,1},1}:
 ['a']
 ['a']
 ['b']
 ['c']
 ['a', 'b']
 ['a', 'b']
 ['a', 'c']
 ['b', 'c']
 ['a', 'b', 'c']
 ['a', 'b', 'c']

Why is the first element of each "increasing size" become doubled?

Doesn't work.
@tpoisot
Copy link

tpoisot commented Aug 9, 2018

This is related to #66, right?


Base.start(c::Combinations) = collect(1:c.t)
function Base.next(c::Combinations, s)
function Base.iterate(c::Combinations)
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than copying the code to a separate method, I believe you could just use Base.iterate(c::Combinations, s = collect(1:c.t)). 🙂

Copy link
Author

Choose a reason for hiding this comment

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

Thanks!!!! For some reason I don't truly understand this actually fixed the problem!

I'll now carry on with the rest.

Copy link
Contributor

Choose a reason for hiding this comment

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

No problem! The default state (second argument) of the iterate function is supposed to be the initial iteration state, which is what start previously provided.

@Datseris
Copy link
Author

hi!!! I just finished this! Please review and merge as soon as you can! :)

@Datseris Datseris changed the title WIP: Fix iterate interface Fix ALL deprecation warnings on Julia 0.7.0 Aug 10, 2018
@ararslan
Copy link
Member

Superseded by #69. Sorry :/

@ararslan ararslan closed this Aug 10, 2018
@Datseris
Copy link
Author

Nooooo come on people why didn't you say you were working on it... :(
Well on the bright side I now fully understand the new iterate system! :)

@HarrisonGrodin
Copy link
Contributor

@Datseris Sorry, didn't realize you were trying to update to 0.7 based on previous PR name... should've inferred. 😞 Nicely done, though.

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.

4 participants