-
Notifications
You must be signed in to change notification settings - Fork 10
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
added limited support for logarithmic units #72
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #72 +/- ##
==========================================
- Coverage 86.11% 83.20% -2.92%
==========================================
Files 1 1
Lines 108 125 +17
==========================================
+ Hits 93 104 +11
- Misses 15 21 +6
Continue to review full report at Codecov.
|
I don't have much time to look at this myself, but could you add one (or more) examples of what this PR adds? |
Nice! Just checking (I'll be able to test this out myself later, just putting it in writing now so I don't forget):
|
Thanks a lot for your considerations!
Please let me know if there is any other functionality that I'm overlooking.. It would be great to have all features working for logarithmic units :) |
I'm not saying, by the way, that this one PR necessarily solves 4. It's nice if it can, but incremental progress is also progress. 1 -- 3 are more important since a user might prefer an error to silently getting an incorrect scaling. It might be worth investigating getting |
@gustaphe I agree with you on all points. I think the best solution is to fix/improve the ustrip and unit functions in Unitful. However, the Unitful code is a bit much for me to solve in my spare time. I found it easier to fix this here; albeit for the time being. With that in mind, I modified the solution by adding two functions My latest commit should solve points 1 -- 3. It, however, does not include sufficient test scripts. I will add some in my next commit. Please wait with this PR until I've added some more tests. Point 4 is also very high on my "want to have" list.. but will require some more effort. |
That's cool. I took the liberty of creating PainterQubits/Unitful.jl#535 yesterday, addressing |
Dear all,
I'd like to contribute to the issue on support for logarithmic units. I've made some modifications and my first simple tests seem promising. Please let me know your thoughts on these modifications.
Kind regards,
Yuri