Address built-in complex deprecation#344
Conversation
|
|
|
@9il That works there, but it looks like I might need to look into |
|
And apparently |
Codecov Report
@@ Coverage Diff @@
## master #344 +/- ##
==========================================
- Coverage 92.13% 92.11% -0.03%
==========================================
Files 63 63
Lines 15232 15248 +16
==========================================
+ Hits 14034 14045 +11
- Misses 1198 1203 +5
Continue to review full report at Codecov.
|
jmh530
changed the title
|
@9il Passing now. Rather than remove the builtin stuff, I just added support for I also added some deprecation notices for |
| } else static if (checkComplex) { | ||
| static if (is(T : cdouble)) { | ||
| alias statType = cdouble; | ||
| import std.complex: Complex; |
There was a problem hiding this comment.
Can this function be reworked with isComplex, realType, TemplateOf , and Unqual without referencing any complex types directly? Would be nice if we don't restrict a user to use Phobos complex type.
|
What you’re arguing is basically to convert the complex types to real,
correct? I’d probably rather not do that. I’d rather just deprecate the
built-in one and then drop support for complex here. The caller can always
convert to real.
…On Mon, May 10, 2021 at 5:05 AM Ilya Yaroshenko ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In source/mir/math/stat.d
<#344 (comment)>:
> } else static if (is(T : double)) {
alias statType = double;
} else static if (checkComplex) {
- static if (is(T : cdouble)) {
- alias statType = cdouble;
+ import std.complex: Complex;
Can this function be reworked with isComplex, realType, TemplateOf , and
Unqual without referencing any complex types directly? Would be nice if
we don't restrict a user to use Phobos complex type.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#344 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADBFNSTWLU4WUUPWDEMB5XDTM6OWXANCNFSM437S4E4Q>
.
|
We can drop support of complex numbers if required right now. I mean that the function shouldn't restrict a user to use std.complex. It should work for arbitrary user-defined templates like |
jmh530
force-pushed
the
change-complex-to-Complex
branch
from
4242516 to
de95a5c
Compare
|
@9il I made some tweaks so that it can handle any type that passes I'm getting some failures on CircleCI. The latest deals with some meson generated files that I'm not sure what's going on. |
| is(F == cfloat) || | ||
| is(F == cdouble) || | ||
| is(F == Complex!float) || | ||
| is(F == Complex!double) || |
There was a problem hiding this comment.
no need to import Phobos here. It can be just isComplex!F && F.sizeof <= 16
| x = s_re + x.im * 1fi; | ||
| } else { | ||
| import std.complex: Complex; | ||
| s = Complex!float(x_re, s.im); |
There was a problem hiding this comment.
No need to import Phobos. Something like F(x_re, s.im) or T(x_re, s.im) should work.
There was a problem hiding this comment.
The implementation shouldn't import std.complex, tests can do that
|
@9il Updated. |
|
Thank you! |
3 participants
This PR addresses the built-in complex deprecation. It currently does not pass CI because
mir.math.sum. The offending line is below. In particular,hasElaborateAssign!Tis true forComplex!Uinstead of false for the built-in types.Not sure how you want to handle (either special case it or just rely on other summation techniques for
Complex).