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
chx wants to merge 1 commit into
hoaproject:masterfrom
chx:master
Closed

Add associative array cartesian products#16
chx wants to merge 1 commit into
hoaproject:masterfrom
chx:master

Conversation

@chx

@chx chx commented Oct 12, 2014

Copy link
Copy Markdown

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.

@fabianx-ai

Copy link
Copy Markdown

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

Hywan commented Nov 3, 2014

Copy link
Copy Markdown
Member

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

@Hywan

Hywan commented Nov 3, 2014

Copy link
Copy Markdown
Member

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

shouze commented Nov 3, 2014

Copy link
Copy Markdown

👍

@chx

chx commented Nov 23, 2014

Copy link
Copy Markdown
Author

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

@chx

chx commented Nov 24, 2014

Copy link
Copy Markdown
Author

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

Hywan commented Nov 24, 2014

Copy link
Copy Markdown
Member

@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

chx commented Nov 24, 2014

Copy link
Copy Markdown
Author

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

Hywan commented Nov 24, 2014

Copy link
Copy Markdown
Member

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

chx commented Nov 24, 2014

Copy link
Copy Markdown
Author

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

@Hywan

Hywan commented Nov 24, 2014

Copy link
Copy Markdown
Member

@chx Perfect :-).

@Hywan

Hywan commented Jan 23, 2015

Copy link
Copy Markdown
Member

@chx ping 😄?

@chx

chx commented Jan 23, 2015

Copy link
Copy Markdown
Author

Sorry! I will do this weekend.

@Hywan

Hywan commented Jan 24, 2015

Copy link
Copy Markdown
Member

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

@chx

chx commented May 10, 2015

Copy link
Copy Markdown
Author

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