-
-
Notifications
You must be signed in to change notification settings - Fork 425
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Scalar.As added BigInteger and Complex #646
Conversation
/// <summary> | ||
/// A collection of operations for working with scalar numeric values. | ||
/// Includes methods like the ones found in <see cref="Math"/> and more. | ||
/// Supports <see cref="Half"/>, <see cref="float"/>, <see cref="double"/>, <see cref="decimal"/>, <see cref="sbyte"/>, <see cref="byte"/>, <see cref="ushort"/>, <see cref="short"/>, <see cref="uint"/>, <see cref="int"/>, <see cref="ulong"/> and <see cref="long"/> |
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.
Generate this comment maybe?
[MethodImpl(MaxOpt)] | ||
public static TTo As<TFrom, TTo>(TFrom val) where TFrom : notnull where TTo : notnull | ||
{ | ||
if (typeof(TFrom) == typeof(Half) && typeof(TTo) == typeof(Half)) |
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.
Why?
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.
Why what?
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.
Why does this exist
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.
What does exist?
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.
Why does this whole self written method exist
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.
Ahhh. It's not method, but now I realized that I can make it a method, and clean the template a bit. Thanks
|
||
#> | ||
<# if (types[i] == "Complex") { #> | ||
return (TTo) (object) (<#= types[j] #>)<#= intermediateType #> ((<#= types[i] #>) (object) val).Real; |
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.
Should add a remark to the docs.
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.
Do we also need to handle the case where TTo is Complex? Or does the explicit conversion already just set the Real?
/// While in most cases the types can be just explicitly casted, | ||
/// sometimes there is an intermediate cast to float or double. |
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.
These shouldn't be part of the summary, but a remark.
Also add another remark that for Complex we only use the Real part
@HurricanKai fixed. Question. I currently include |
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.
One last nitpick.
I think keeping it like this is fine otherwise.
Co-authored-by: Kai Jellinghaus <contact@kaij.tech>
* Scalar.As added BigInteger and Complex Now T4-backed * but i included those files???? * Working on intermediate casts * Fixing intermediate casts * PR review fixes * PR comments * Update src/Maths/Silk.NET.Maths/Scalar.As.tt Co-authored-by: Kai Jellinghaus <contact@kaij.tech> * Ran the template Co-authored-by: Kai Jellinghaus <contact@kaij.tech>
Now T4-backed
Closes #639