Merged
Conversation
- [ ] 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)
fa09704 to
1cb826d
Compare
Contributor
|
Sounds good to me. Better to evaluate these kinds of expressions in native Nu code. |
Member
Author
|
Making a deprecation note for a subcommand currently kind of sucks: |
`math eval` now under `nu-command/src/deprecated` This gives a deprecation warning when opening the help and guides you if you use the bare command. But in original usage the deprecation warning is useless as you get an error about the number of arguments. We should do better
…hell into remove-math-eval
Member
Author
|
Not happy with the state of deprecation yet #7530 |
Contributor
|
Nice, thanks for getting this over the line. |
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
2 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
Reasoning:
Most missing math commands are implemented with #7258.
The
mevalcrate itself declares that it doesn't strive to stringentstandards (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
stdlibrary 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
mathcommands can themselves implement broadcastingover lists (or table columns).
Closes #7073
Removed dependencies:
mevalnom 1.2.4(duplicate)User-Facing Changes
Scripts using
math evalwill break.We remove a further
evallike behavior to get results through runtime evaluation (albeit limited in scope)Tests
math eval.math evalto obtain a result fromstr join