Implement more math commands (e.g. trig, ln)#7258
Merged
sholderbach merged 6 commits intonushell:mainfrom Dec 1, 2022
Merged
Conversation
8b2b09c to
b78b060
Compare
sholderbach
added a commit
to sholderbach/nushell
that referenced
this pull request
Nov 29, 2022
- [ ] Requires sign off from the team Reasoning: Most missing math commands are implemented with nushell#7258. The `meval` crate itself declares that it doesn't strive to stringent standards (https://docs.rs/meval/latest/meval/#related-projects). For example no particular special casing or transformations are performed to ensure numerical stability. It uses the same rust `std` library functions we use or have access to (and `f64`). While the command call syntax in nushell may be a bit more verbose, having a single source of truth and common commands is beneficial. Furthermore the `math` commands can themselves implement broadcasting over lists (or table columns). Closes nushell#7073 Removed dependencies: - `meval` - `nom 1.2.4` (duplicate)
sholderbach
added a commit
to sholderbach/nushell
that referenced
this pull request
Nov 29, 2022
- [ ] Requires sign off from the team Reasoning: Most missing math commands are implemented with nushell#7258. The `meval` crate itself declares that it doesn't strive to stringent standards (https://docs.rs/meval/latest/meval/#related-projects). For example no particular special casing or transformations are performed to ensure numerical stability. It uses the same rust `std` library functions we use or have access to (and `f64`). While the command call syntax in nushell may be a bit more verbose, having a single source of truth and common commands is beneficial. Furthermore the `math` commands can themselves implement broadcasting over lists (or table columns). Closes nushell#7073 Removed dependencies: - `meval` - `nom 1.2.4` (duplicate)
fdncred
reviewed
Dec 1, 2022
Contributor
|
Lots of repetitive code here, as is the way when adding a bunch of commands. I skimmed through these, mainly looking for copy-n-paste errors and such, and none jumped out at me. I think we should move ahead with landing these and your other math pr too, and then take a look at combining all this stuff in a math plugin. I think it's the only way we'll be able to do any meaningful perf testing to determine if moving to a plugin is the right thing to do. |
Member
Author
|
Thanks for going through all the boilerplate @fdncred! Should I rebase the blocks of commands into their own commits so that removing them is easier if we decide to move them to a plugin? |
Contributor
That sounds like a pain. I probably wouldn't. |
Currently implemented as commands Work towards nushell#7073 Add them to the example test harness together with `math round`
`math sin` `math cos` `math tan` Support degrees with the flag `--degrees`/`-d`
`math sinh` `math cosh` `math tanh`
- `math arcsin` - `math arccos` - `math arctan` Again include `--degrees` or `-d` for convenient output in degrees
- `math arcsinh` - `math arccosh` - `math arctanh`
- `math ln`
1a38cc5 to
2009a7b
Compare
Member
Author
I was a masochist in this case. |
sholderbach
added a commit
that referenced
this pull request
Dec 1, 2022
sholderbach
added a commit
that referenced
this pull request
Dec 1, 2022
`math sin` `math cos` `math tan` Support degrees with the flag `--degrees`/`-d`
sholderbach
added a commit
that referenced
this pull request
Dec 1, 2022
`math sinh` `math cosh` `math tanh`
sholderbach
added a commit
that referenced
this pull request
Dec 1, 2022
- `math arcsin` - `math arccos` - `math arctan` Again include `--degrees` or `-d` for convenient output in degrees
sholderbach
added a commit
that referenced
this pull request
Dec 1, 2022
- `math arcsinh` - `math arccosh` - `math arctanh`
sholderbach
added a commit
that referenced
this pull request
Jan 4, 2023
Reasoning: Most missing math commands are implemented with #7258. The `meval` crate itself declares that it doesn't strive to stringent standards (https://docs.rs/meval/latest/meval/#related-projects). For example no particular special casing or transformations are performed to ensure numerical stability. It uses the same rust `std` library functions we use or have access to (and `f64`). While the command call syntax in nushell may be a bit more verbose, having a single source of truth and common commands is beneficial. Furthermore the `math` commands can themselves implement broadcasting over lists (or table columns). Closes #7073 Removed dependencies: - `meval` - `nom 1.2.4` (duplicate) User-Facing Changes: Scripts using `math eval` will break. We remove a further `eval` like behavior to get results through runtime evaluation (albeit limited in scope) Tests: - Updated tests that internally used `math eval`. - Removed one test that primarily used `math eval` to obtain a result from `str join`
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Work towards #7073
math piandmath econstantsmath sinmath cosmath tanmath sinhmath coshmath tanhmath arcsinmath arccosmath arctanmath arcsinhmath arccoshmath arctanhmath lnTests + Formatting
math pi,math e, andmath round