mathjs icon indicating copy to clipboard operation
mathjs copied to clipboard

Fraction numerator denominator helpers

Open AnslemHack opened this issue 1 month ago • 4 comments

This PR fixes #3595 introduces two new arithmetic functions, num() and den(), which extract the numerator and denominator from Fraction objects respectively.

AnslemHack avatar Dec 02 '25 23:12 AnslemHack

Thanks for working out these functions num and den @AnslemHack . Your PR looks well taken care of!

I made a few inline comments, can you have a look at those?

On a side note: the detailed description of your PR looks AI-generated and doesn't really add useful information to me 😅. I think it's enough to explain that "this PR addresses #3595, implementing two new functions, num and den".

my apologise for the verbose @josdejong , I must have over refined my output description, I have now made changes to that with a brief description of what it simply does, hope this is better

AnslemHack avatar Dec 03 '25 15:12 AnslemHack

Thanks!

josdejong avatar Dec 03 '25 16:12 josdejong

A few more items:

  • re() and im() are in src/function/complex. Shouldn't these be in a new subdirectory src/function/fraction? (Jos?)
  • Please add some tests showing that num and den do reasonable things on number and bigint input.
  • Presumably at the moment num and den will fail on BigNumber input, because there is no implicit conversion from BigNumber to Fraction. But it seems to me that for these two particular functions, what else could one mean but to convert the BigNumber to Fraction and then return the numerator or denominator? So, with @josdejong's blessing, please add an explicit signature for BigNumber for both functions that does the conversion and then returns the numerator or denominator. And then tests for this behavior, of course. Thanks!

gwhitney avatar Dec 04 '25 05:12 gwhitney

A few more items:

  • re() and im() are in src/function/complex. Shouldn't these be in a new subdirectory src/function/fraction? (Jos?)
  • Please add some tests showing that num and den do reasonable things on number and bigint input.
  • Presumably at the moment num and den will fail on BigNumber input, because there is no implicit conversion from BigNumber to Fraction. But it seems to me that for these two particular functions, what else could one mean but to convert the BigNumber to Fraction and then return the numerator or denominator? So, with @josdejong's blessing, please add an explicit signature for BigNumber for both functions that does the conversion and then returns the numerator or denominator. And then tests for this behavior, of course. Thanks!

thanks for your feedback I have now addressed your concerns. please take another look.

AnslemHack avatar Dec 06 '25 20:12 AnslemHack