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

No links to "construction" doc pages #3368

Open
gwhitney opened this issue Jan 27, 2025 · 8 comments · May be fixed by #3375
Open

No links to "construction" doc pages #3368

gwhitney opened this issue Jan 27, 2025 · 8 comments · May be fixed by #3375
Labels
bug documentation Concerns about or enhancements to the mathjs documentation

Comments

@gwhitney
Copy link
Collaborator

Describe the bug
There are doc pages for construction functions such as bignumber but no links to such functions from the function reference page. Further, the new construction function bigint has no page.

To Reproduce
n/a

Discussion
I plan to address this in the PR for #3366 which will also update the bigint function.

@gwhitney gwhitney added bug documentation Concerns about or enhancements to the mathjs documentation labels Jan 27, 2025
gwhitney added a commit to gwhitney/mathjs that referenced this issue Jan 31, 2025
   Adds options to the bigint constructor function `math.bigint()` to
   control whether/how non-integer inputs are rounded or errors thrown,
   and whether "unsafe" values outside the range where integers can be
   uniquely represented are converted. Changes `math.numeric(x, 'bigint')`
   to call the bigint constructor configured to allow unsafe values but
   throw on non-integers (closest approximation to its previous buggy
   behavior).

   Also restores documentation generation for constructor functions,
   and initiates History sections in function doc pages.

   Resolves josdejong#3366.
   Resolves josdejong#3368.
   Resolves josdejong#3341.
@gwhitney gwhitney linked a pull request Jan 31, 2025 that will close this issue
@josdejong
Copy link
Owner

Thanks, good idea to create a page for bigint like there is for the other data types.

@gwhitney
Copy link
Collaborator Author

Yes but also the constructor pages are currently "lost" -- not linked to from the function reference page on mathjs.org. that also needs fixing, and I think #3375 does so.

@josdejong
Copy link
Owner

The class desciptions can be found via Docs > Reference > Classes, is that what you mean?

@gwhitney
Copy link
Collaborator Author

No. I mean the doc pages for the functions fraction or complex or even number have been lost from the mathjs.org site. They are simply not linked on the function reference page-- the whole "construction" section has gone missing. It is restored in PR #3375.

@josdejong
Copy link
Owner

Ah, I now I understand what you mean, you're right.

What do you think is best, address this in #3375 or put together a small PR that fixes just this issue only?

@gwhitney
Copy link
Collaborator Author

Ah, I now I understand what you mean, you're right.

What do you think is best, address this in #3375 or put together a small PR that fixes just this issue only?

Well, #3375 is currently hung up on a design question: how to support a client wanting to convert a JavaScript number to a bigint, but only when the number is a "safe" integer? Note there is currently no implicit conversion number => bigint in mathjs, and it would probably be problematic to add one, as there is an implicit conversion bigint => number (that does check for "safety") and we have generally run into troubles with bidirectional implicit conversions. Also, implicit conversions don't help with a user-supplied mathjs expression language formula that happens to return number but the code using it wants a bigint -- it has to wrap the formula in something to make the conversion happen.

So the conversion must be requested explicitly, and I think there is a strong use case for a "safe" conversion option. In our case, we are doing number theory calculations, including modular arithmetic that gets thrown off badly when a JavaScript number exceeds the "safe" range, and so we need protection from that happening.

The other debate in #3375 is the per-function 'History' section: you're worried that convention would be hard to maintain, whereas I feel it is much easier to maintain than the existing central 'History', since a function's History only needs an update when you touch its source file, and if you are editing the code, it's easy to just add a line to the History right there, and it's easy in review to check that any function source file that was modified as to behavior had a History line added.

So if you see those as being resolvable fairly soon/fairly easily (if you provide direction/decisions on them, e.g, you want a CI workflow addition that checks for new History lines in edited function files, I am happy to update #3375 right away, since it is blocking our adoption of mathjs v14+), then it seems easiest to just go with #3375. If you see them as more protracted, then just let me know and I will extract the portion of it that restores these doc pages as a standalone PR and rebase #3375 once that's merged.

My apologies for folding three things into #3375 (bigint conversion, per-function History, and reviving "construction" doc pages). I recognize that's not typically development "best practice" but given how many outstanding items there are on the mathjs list it can be the most efficient time-wise, when it goes smoothly. So I succumbed to the temptation to do a few things "while I was there" because of my feeling of the pressure of a very long list of mathjs needs (e.g. 100+ open issues and many PRs hanging around and a lot of Discussions with really good ideas besides).

@josdejong
Copy link
Owner

👍 I've repied in #3375 (comment)

@josdejong
Copy link
Owner

My apologies for folding three things into #3375 (bigint conversion, per-function History, and reviving "construction" doc pages).

Yeah it's not ideal. Making a PR addressing multiple things in one go is easier for the one creating the PR, but harder for the one that has to review the PR 😅.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug documentation Concerns about or enhancements to the mathjs documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants