UnitfulRecipes.jl
UnitfulRecipes.jl copied to clipboard
limited support for logarithmic units including dB*m and dB/Hz
I hope you don't me starting a new pull request on logarithmic units. I decided on a new approach to support logarithmic units. This requires minimal changes to the existing code, but adds support for the four identified issues:
- What happens if you plot
dBV
and thenplot!
dBμV
? -> works - If you
plot
dBV
, and thenplot!
V
? -> works - If you plot
V
and thenplot!
dBV
? -> works - Is
dB/m
treated well? -> dB/m, dB/Hz, dB*m are working
Note: I found that only the unit
function in Unitful.jl required a fix. See the comments in the UnitfulWrapper.jl file. unit
produces inconsistent results for logarithmic units. Fixing this within Unitful.jl caused a lot of things to break (a lot of math in Unitful requires unit to behave the way it does). As a temporary fix, I took the liberty of defining a new function fullunit
which returns an appropriate unit instead.
Finally, Unitful.jl does not have a method ustrip(unit, quantity)
for logarithmic units. I added one in the UnitfulWrapper.jl.
These changes are sufficient to support logarithmic units.
I will be very grateful if you'd consider merging this PR. Let me know if you have any thoughts or comments on the solution. Improvements are also welcome.
Codecov Report
Merging #73 (da05310) into master (759bf85) will increase coverage by
1.19%
. The diff coverage is71.42%
.
@@ Coverage Diff @@
## master #73 +/- ##
==========================================
+ Coverage 86.11% 87.30% +1.19%
==========================================
Files 1 2 +1
Lines 108 126 +18
==========================================
+ Hits 93 110 +17
- Misses 15 16 +1
Impacted Files | Coverage Δ | |
---|---|---|
src/UnitfulRecipes.jl | 86.48% <55.00%> (+0.37%) |
:arrow_up: |
src/UnitfulWrapper.jl | 93.33% <93.33%> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 7741be7...da05310. Read the comment docs.
Ok, I've added some test for the fullunit
function. This should improve the code coverage to an acceptable level.