Skip to content
This repository was archived by the owner on Sep 20, 2021. It is now read-only.

Add associative array cartesian products #16

Closed
wants to merge 1 commit into from
Closed

Conversation

chx
Copy link

@chx chx commented Oct 12, 2014

This is a fascinating library to calculate cartesian products without recursion and without keeping the whole set in memory. May I ask for associative array support? If this is not desired, could you at least please consider splitting off the init function so I can add my subclass without copying the whole constructor? Thanks.

@LionsAd
Copy link

LionsAd commented Oct 12, 2014

This looks great to me. ( I peer reviewed it). It would be really great to get this in.

@Hywan Hywan self-assigned this Nov 3, 2014
@Hywan
Copy link
Member

Hywan commented Nov 3, 2014

Sorry for the delay, I was on vacations :-).

@Hywan
Copy link
Member

Hywan commented Nov 3, 2014

Since the library is not yet finalized (still in development), we can change the behavior of the cartesian product iterator. Having associated keys sounds good for me. Can you update your patch to modify Hoa\Math\Combinatorics\Combination\CartesianProduct accordingly instead of creating a new class?

I am asking the opinion of @stephpy or @shouze also.

@shouze
Copy link

shouze commented Nov 3, 2014

👍

@chx
Copy link
Author

chx commented Nov 23, 2014

Sorry for not yet rolling this; I will get back to you soon.

@chx
Copy link
Author

chx commented Nov 24, 2014

BTW http://cgit.drupalcode.org/sandbox-chx-1857558/tree/core/lib/Drupal/Component/Utility/CartesianProduct.php?h=router-new the code is here; the test is http://cgit.drupalcode.org/sandbox-chx-1857558/tree/core/tests/Drupal/Tests/Component/Utility/CartesianProductTest.php?h=router-new here. The code is my work you are free to take it especially because it is very clearly inspired by yours.

The constructor takes a different argument if I remember correctly.

@Hywan
Copy link
Member

Hywan commented Nov 24, 2014

@chx If the code is inspired by Hoa, I would ask you a mention if possible :-).
Do we wait you to update the PR or do we continue by ourselves?

@chx
Copy link
Author

chx commented Nov 24, 2014

That code won't be in Drupal, I believe, it was an aborted attempt, that's why it's in a sandbox. If you can continue from here, please do. I have no problems with a joint authorship -- the basic idea of using iterators for each item and the underlying logic of implementing the advancing loop is certainly yours. The implementation, on the other hand, is mostly mine :)

@Hywan
Copy link
Member

Hywan commented Nov 24, 2014

I prefer you to continue since we have few resources here ;-). If you don't have time, tell it cleary and we will find time to continue your work and proposal :-).

@chx
Copy link
Author

chx commented Nov 24, 2014

I can adjust this for you but I can only do so next weekend.

@Hywan
Copy link
Member

Hywan commented Nov 24, 2014

@chx Perfect :-).

@Hywan
Copy link
Member

Hywan commented Jan 23, 2015

@chx ping 😄?

@chx
Copy link
Author

chx commented Jan 23, 2015

Sorry! I will do this weekend.

@Hywan
Copy link
Member

Hywan commented Jan 24, 2015

No hurry :-). It was a simple ping ;-).

@chx
Copy link
Author

chx commented May 10, 2015

Closing in favor of #23

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

4 participants