tcbrindle / flux

A C++20 library for sequence-orientated programming
https://tristanbrindle.com/flux/
Boost Software License 1.0
484 stars 29 forks source link

`cartesian_power`, `cartesian_power_map` #162

Closed isaacy2012 closed 8 months ago

isaacy2012 commented 8 months ago

Attempts to partially address #138 regarding the cartesian_power, cartesian_power_map functionalities.

Implementation centers around replacing std::get<I>(self.bases_) in cartesian_product with self.base_ for cartesian_power.

Moves common logic across cartesian_product, cartesian_power and their map variants to a common base trait class cartesian_base with enum selectors for [product/power] and [tuple/map].

codecov[bot] commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (19e87a3) 97.99% compared to head (1902bb8) 98.06%.

:exclamation: Current head 1902bb8 differs from pull request most recent head b8b59f0. Consider uploading reports for the commit b8b59f0 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #162 +/- ## ========================================== + Coverage 97.99% 98.06% +0.06% ========================================== Files 66 69 +3 Lines 2392 2424 +32 ========================================== + Hits 2344 2377 +33 + Misses 48 47 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

isaacy2012 commented 8 months ago

unless you have any other suggestions?

Not really, but I got cartesian_power from this wikipedia page: https://en.wikipedia.org/wiki/Cartesian_product

I've changed it back to cartesian_product_repeat though, as if people are used to the Python way then it's probably better than the technically correct but potentially confusing power.

tcbrindle commented 8 months ago

unless you have any other suggestions?

Not really, but I got cartesian_power from this wikipedia page: https://en.wikipedia.org/wiki/Cartesian_product

I've changed it back to cartesian_product_repeat though, as if people are used to the Python way then it's probably better than the technically correct but potentially confusing power.

Hmm, I originally just googled the words cartesian power and it returned lots of results about cartesian products and power sets, which is why I was initially reluctant.

But now I've just searched again, using "cartesian power" (with quotes) and I see that actually the term is used in a few places -- for example Microsoft's Azure Quantum language, as well as some sites explaining mathematical terminology.

So if it's the right term mathematically, and it has precedent in other places, and it has a nice definition we can point to in the documentation.... then I guess cartesian_power is what we should call it after all?

I feel bad asking you to change it back again! But I see that it was all in the most recent commit so hopefully it's easy to revert. Sorry for getting you to do unnecessary work, but you've convinced me that you were right all along :)

isaacy2012 commented 8 months ago

I feel bad asking you to change it back again! But I see that it was all in the most recent commit so hopefully it's easy to revert. Sorry for getting you to do unnecessary work, but you've convinced me that you were right all along :)

No worries, I've renamed the branch to reflect the finalised name, so I'll make the updated PR from that branch.