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.

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

feat(array): cartesianProduct #241

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

yamcodes
Copy link
Contributor

@yamcodes yamcodes commented Sep 10, 2024

Tip

The owner of this PR can publish a preview release by commenting /publish in this PR. Afterwards, anyone can try it out by running pnpm add radashi@pr<PR_NUMBER>.

Summary

Add a cartesianProduct function for arrays.

NOTE: Unfortunately, this only supports a single type. Mixed types are not fully supported, for example, _.product([[1,2],["hello","world"]]) will throw a type error. Fully mixed types are tricky to achieve in TypeScript for Cartesian Products, because of how things get "mixed up". In the above example, we know the result is composed of arrays like [1, "world"] with type [number, string]. I haven't found a good way to solve it. See discussion here: https://gist.github.com/ssippe/1f92625532eef28be6974f898efb23ef

Thanks to @MarlonPassos-git we now support mixed types! ❤️

Related issue, if any:

For any code change,

  • Related documentation has been updated, if needed
  • Related tests have been added or updated, if needed
  • Related benchmarks have been added or updated, if needed

Does this PR introduce a breaking change?

No

Bundle impact

Status File Size 1
A src/array/cartesianProduct.ts 164

Footnotes

  1. Function size includes the import dependencies of the function.

@MarlonPassos-git
Copy link
Contributor

@yamcodes thanks for the PR. some considerations.

  1. I really don't understand why you have to pass [...any[]] instead of using the spread operator "..."
export function product<T>(...arrays: T[]): T[][] {
  1. about types, I think I found an implementation that solves the type problem
export function product<T extends any[]>(...arrays:  [...T]): Array<{[K in keyof T]: T[K][number]}> 
export function product<T extends any[]>(...arrays: T): T[][] {
  let out: T[][] = [[]]
  for (const array of arrays) {
    const result: T[][] = []
    for (const currentArray of out) {
      for (const item of array) {
        const currentArrayCopy = currentArray.slice()
        currentArrayCopy.push(item)
        result.push(currentArrayCopy)
      }
    }
    out = result
  }
  return out
}

image
image
image

@MarlonPassos-git
Copy link
Contributor

and since it is now possible to have better types, I recommend creating a test for the types, a suggestion:

import * as _ from 'radashi'

describe('product types', () => {
  test('with an empty array', () => {
    const result = _.product([])
    expectTypeOf(result).toEqualTypeOf<[never][]>()
  })
  test('with two arrays of the same type', () => {
    const result = _.product(['red', 'blue'], ['fast', 'slow'])
    const [[v1, v2]] = result
    expectTypeOf(result).toEqualTypeOf<[string, string][]>()
    expectTypeOf(v1).toEqualTypeOf<string>()
    expectTypeOf(v2).toEqualTypeOf<string>()
  })
  test("with two arrays of different types", ()=> {
    const result = _.product(['red', 'blue'], [1, 2])
    const [[v1, v2]] = result
    expectTypeOf(result).toEqualTypeOf<[string, number][]>()
    expectTypeOf(v1).toEqualTypeOf<string>()
    expectTypeOf(v2).toEqualTypeOf<number>()
  })
  test("with three arrays of different types", ()=> {
    const result = _.product(['red', 'blue'], [1, 2], [true, false])
    const [[v1, v2, v3]] = result
    expectTypeOf(result).toEqualTypeOf<[string, number, boolean][]>()
    expectTypeOf(v1).toEqualTypeOf<string>()
    expectTypeOf(v2).toEqualTypeOf<number>()
    expectTypeOf(v3).toEqualTypeOf<boolean>()
  })
  test("with constant arrays of different types", ()=> {
    const result = _.product(['red', 'blue'] as const, [1, 2] as const)
    const [[v1, v2]] = result
    expectTypeOf(result).toEqualTypeOf<["red" | "blue", 1 | 2][]>()
    expectTypeOf(v1).toEqualTypeOf<"red" | "blue">()
    expectTypeOf(v2).toEqualTypeOf<1 | 2>()
  })
  test("with a mix of constant and non-constant types", ()=> {
    const result = _.product(['red', 'blue'], [1, 2] as const)
    const [[v1, v2]] = result
    expectTypeOf(result).toEqualTypeOf<[string, 1 | 2][]>()
    expectTypeOf(v1).toEqualTypeOf<string>()
    expectTypeOf(v2).toEqualTypeOf<1 | 2>()
  })
})

tests/array/product.test.ts Outdated Show resolved Hide resolved
src/array/product.ts Outdated Show resolved Hide resolved
src/array/product.ts Outdated Show resolved Hide resolved
src/array/product.ts Outdated Show resolved Hide resolved
docs/array/product.mdx Outdated Show resolved Hide resolved
@MarlonPassos-git MarlonPassos-git added the new feature This PR adds a new function or extends an existing one label Sep 10, 2024
yamcodes and others added 3 commits September 11, 2024 22:43
Co-authored-by: Marlon Passos <1marlonpassos@gmail.com>
+ Also use `castArray` instead of `.slice()` for shallow copy
src/array/product.ts Outdated Show resolved Hide resolved
@aleclarson
Copy link
Member

Thanks for the PR! I think naming the function cartesianProduct would make its purpose clearer to anyone reading the code. The term "product" can be a bit ambiguous since it could also refer to element-wise multiplication or dot products. Using cartesianProduct would help avoid any confusion and match the common terminology for this operation. What do you think?

@yamcodes
Copy link
Contributor Author

yamcodes commented Sep 11, 2024

@MarlonPassos-git

  1. I also prefer supplying the arrays as parameters, so we should go with your solution. I'll add that I followed the example set by unzip, so it'd be a good idea to update that as well. @aleclarson
  2. Oh. My. God. I'm so blown away!!! The usage of:

a. Spreading generic tuple parameters ([...T])
b. Overloads to simplify the implementation (instead of type assertions)
c. Mapped types on tuples

Is pure genius. I've learned something new today because I had no idea you could spread generic types. Thank you, Marlon!

@aleclarson Thanks! It's a pleasure to join your community and finally contribute!
I also prefer cartesianProduct, since it eliminates a cognitive overhead, so I'm glad you brought it up. I just got the impression that the library prefers shorter and "cuter" names rather than explicit ones. I'll go ahead and rename.

+ Add type tests
+ Update tests to support multiple types
+ Update doc to display multiple types
@yamcodes yamcodes changed the title feat(array): product feat(array): cartesianProduct Sep 11, 2024
@yamcodes
Copy link
Contributor Author

yamcodes commented Sep 11, 2024

Guys, I've updated the PR to reflect your great suggestions. Let me know what you guys think of it as a whole!

benchmarks/array/cartesianProduct.bench.ts Outdated Show resolved Hide resolved
tests/array/cartesianProduct.test.ts Outdated Show resolved Hide resolved
src/mod.ts Outdated Show resolved Hide resolved
@yamcodes
Copy link
Contributor Author

Suggestions were applied and I've also enforced that T extends any[][] to enforce the input parameters to all be arrays.

Copy link
Contributor

@MarlonPassos-git MarlonPassos-git left a comment

Choose a reason for hiding this comment

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

LGTM

@radashi-bot
Copy link

Benchmark Results

Name Current
cartesianProduct: with an empty array (n=1) 3,877,380.81 ops/sec ±0.33%
cartesianProduct: with a non-empty array (n=1) 2,157,966.38 ops/sec ±0.51%
cartesianProduct: with two small arrays (n=2) 1,713,313.76 ops/sec ±0.16%
cartesianProduct: with three small arrays (n=3) 1,060,986.87 ops/sec ±0.65%

Performance regressions of 30% or more should be investigated, unless they were anticipated. Smaller regressions may be due to normal variability, as we don't use dedicated CI infrastructure.

@aleclarson aleclarson added this to the v12.3.0 milestone Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature This PR adds a new function or extends an existing one
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants