Conversion now has both a value and a factor type #453
Conversation
|
Thanks for the PR. I kicked off the CI checks and will try to review this weekend. |
|
Should i rebase this and try again? Or do you have a different solution/idea/want for this? Not sure when i can schedule time again, but with any luck i can get it in before the weekend. |
mkalte666
force-pushed
the
complex_number_conversion
branch
from
b285dc8
to
8d36211
Compare
|
Heya, i have rebased this to master and will put it into local testing again. Can you have a look at it at some point in the future @iliekturtles? Or if you need any input from my end (or me to come up with a better way of solving my complex number issue) i am also happy to help out, but this PR (excluding the rebase, working on reporting on it) has been working for me so far so i have not put too much more thought into this x.x EDIT: up there is me trying and failing to ask/say: "hey, if you do need more input/work done and cant do it yourself, feel free to ask.". And additionally, if you simply cannot spare the time, that is also fine as well, please do not feel pressured. I just hope to give back a bit to the things i use and do at work, but i very much do not want to force myself on projects with "hey do this PR" like comments. Reflecting on this, i think kind of sounded like that up there. So sorry for that x.x |
Initially, `ConversionFactor` and thus `Conversion::T` requred `PartialEq`. This makes sense for the conversion factor itself (i.e. scaling across units), however it breaks once you introduce complex numbers. Those can *still* be scaled just like normal numbers - you essentially just increase or decrese a vector length, but the conversion function cannot compare them - "Z_1 < Z_2" is not trivially decidable. It is, however, also not needed - unit scales are just that - scalars that scale. And those can be easily compared. This commit seperates `Conversion::T` into `Conversion::VT` and `Conversion::T` and moves the `PartialEq` requirements from `ConversionFactor` into `Conversion::TT` directly. This requires a lot of trait bounds added down the line, so im not 100% that this does not break anything down the line. There might be a nicer way to go about this, but i haven't found any. closes iliekturtles#452
mkalte666
force-pushed
the
complex_number_conversion
branch
from
8d36211
to
2a8261a
Compare
Initially,
ConversionFactorand thusConversion::TrequredPartialEq. This makes sense for the conversion factor itself (i.e. scaling across units), however it breaks once you introduce complex numbers.Those can still be scaled just like normal numbers - you essentially just increase or decrese a vector length, but the conversion function cannot compare them - "Z_1 < Z_2" is not trivially decidable.
It is, however, also not needed - unit scales are just that - scalars that scale. And those can be easily compared.
This commit seperates
Conversion::TintoConversion::VTandConversion::Tand moves thePartialEqrequirements fromConversionFactorintoConversion::TTdirectly.This requires a lot of trait bounds added down the line, so im not 100% that this does not break anything down the line.
There might be a nicer way to go about this, but i haven't found any.
closes #452
Please have a look and see if this is where it should go.
Things that still need to be done: