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

types: quantileSeq expands types unnecessarily #3197

Closed
domdomegg opened this issue Apr 28, 2024 · 5 comments · Fixed by #3198
Closed

types: quantileSeq expands types unnecessarily #3197

domdomegg opened this issue Apr 28, 2024 · 5 comments · Fixed by #3198

Comments

@domdomegg
Copy link
Contributor

Describe the bug

The types for quantileSeq have an overly wide return type.

To Reproduce

// Type 'number | BigNumber | Unit | MathArray' is not assignable to type 'number'.
//   Type 'BigNumber' is not assignable to type 'number'.ts(2322)
const value: number = math.quantileSeq([1, 2, 3], 0.90);

I think value here should always be a number. This is the behaviour of for example math.mean and math.median.

@josdejong
Copy link
Owner

Thanks for your input, we could indeed improve the TypeScript definitions I think. Help would be welcome.

@josdejong
Copy link
Owner

Fixed now in v12.4.3

@Loksly
Copy link

Loksly commented Jun 21, 2024

Hi. This fix https://github.com/josdejong/mathjs/pull/3198/files broke basic usage:

Now this basic usage is broken.

const quantiles: number[] = math.quantileSeq([1, 2, 3, 4, 5], [0.25, 0.75]) as number[];

This was supported previous to v12.4.3.

@domdomegg
Copy link
Contributor Author

Thanks @Loksly. I've raised a PR to fix this: #3223

In the mean time this workaround should work:

const quantiles: number[] = math.quantileSeq([1, 2, 3, 4, 5], [0.25, 0.75]) as unknown as number[];

@josdejong
Copy link
Owner

This fix is now published in v13.0.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants