Flexible fullSimplify #35
Closed
Conversation
This file contains 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
sharkdp
requested changes
Sep 28, 2018
| @@ -136,12 +137,16 @@ toStandard (num .*. du) = | |||
| case U.toStandardUnit du of | |||
| Tuple du' conversion → (conversion * num) ..* du' | |||
|
|
|||
| -- | Attempt to simplify the unit of a quantity. | |||
| -- | Attempt to simplify the unit of a quantity, except angles. | |||
There was a problem hiding this comment.
To be precise, the docstring should be something like:
Attempt to simplify the unit of a quantity. Quantities with units that be completely cancelled will be reduced to scalar values (except for angle-units)
.. and similar for the fullSimplify' function below.
| fullSimplify = fullSimplify' [SI.degree, SI.radian, SI.steradian] | ||
|
|
||
| -- | Attempt to simplify the unit of a quantity, except the given units. | ||
| fullSimplify' ∷ Array DerivedUnit → Quantity → Quantity |
There was a problem hiding this comment.
If you want to use this function, it should be exported(?)
|
@abaco Ping on this |
|
Feel free to comment here if you want to pick this up again. Going to close this for now to clean up my open-PR list. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
fullSimplifyattempts to simplify the unit of a quantity, except degrees and radians. With this PR I've addedfullSimplify', which takes an array of units the shouldn't be simplified.I need this for percentages, which I defined as a "dimensionless" unit.
In the process I've also added steradian to the units that aren't simplified - is that fine or was it left out on purpose?