-
Notifications
You must be signed in to change notification settings - Fork 387
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
Dynamic unit conversions #588
Conversation
… UnitConverter. Adding test for custom direct conversions
This isn't completely fleshed out yet. Just to show an idea, so don't merge it yet. Comments in #478 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting approach, it definitely unlocks the ability to add explicit unit-to-unit conversions even across quantities. I'll comment further in the issue #478 instead.
UnitsNet/GeneratedCode/Quantities/ElectricInductance.NetFramework.g.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New changes looks good, but still need to digest how and why this will be used. Continuing discussion in issue.
...t.WindowsRuntimeComponent/GeneratedCode/Quantities/Acceleration.WindowsRuntimeComponent.g.cs
Outdated
Show resolved
Hide resolved
Adding a summary from prior discussion here: As for direct conversions, I think that sounds like a good idea regardless:
|
Re-reading this now. |
It was looked up and wrapped N times
They became a bit unwieldy. Move fields to top of class.
Since this is the most commonly used call path by far, it made sense to handle this case separately. The advantage is that the callback quantity is strongly typed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is looking pretty good. I just took the liberty of adding some changes directly to the branch, please review them and see what you think.
When this is merged, this wiki should be updated on the new capability to add unit conversions: |
More work to do here (including doc obviously after warnings-as-errors). Just merging to keep up to date. |
…m quantity conversions
@tmilnthorp I took this for spin locally and added some improvements. I think this is ready to merge now, but waiting for you to check out the latest changes. I was specifically interested in seeing how we can improve support for third party quantities and units and added some test cases for that. It works pretty well by now, you can parse units, parse quantities and convert between quantities using only custom types. We still enforce the requirement of implementing I think there is definitely an opportunity to support simple POCO classes and structs without them having to implement If you have time, I'd also love to merge #656 . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, waiting for you to review my latest improvements.
This needs to port the powershell changes to the new C# codegen scripts. Update: Fixed. |
The only difference is fixing the header of a file.
@tmilnthorp I think this one is good to go. Two months ago I made some changes to it and I'd like you to look over them whenever you find the time so we can get this puppy merged :) |
Looks good to me! |
Sweet! |
Idea for #478. I'll add more commentary there.