implement OpAssign for Complex

[bluss]

Use a crate feature ("opassign")

Design choice: Use by-value forms of OpAssign and use Clone to forward the reference arguments. That matches what the other traits do (for example Add).

Design choice: The complex += complex operation needs Add, not just AddAssign. The complex += real operation can use just AddAssign. You could just use Add for both, not sure which way is preferable.

[bluss]

OpAssign is possibly a big break inducer higher up in num as well.

You could consider adding an extra trait like this: trait NumOpAssign: Num + AddAssign + SubAssign + .., and then use that in impl AddAssign in complex, the same way Num is used for impl Add.

That alone didn't seem like a great motivation to introduce "NumOpAssign."

[cuviper]

Instead of a manual feature, perhaps we could use rustc_version in a build script to add --cfg opassign when supported. (version_matches(">= 1.8.0"))

Implementing these by value is acceptable to me. Most of the time I expect Complex<T> is used with simple primitives anyway, especially floats.

The trait requirements are the tricky question, because we can never expand them without a breaking change. For instance, in AddAssign I probably would have used += on the real and imaginary parts, ditto SubAssign, but we can never switch to that unless we include those requirements now. Maybe we should just add those particular cases now.

I also think it will be confusing to users if opassign is allowed with a complex RHS, but not a real. e.g. if complex_a *= real_b fails but complex_a *= Complex::from(real_b) would succeed. It's clear why the implementation happened that way, and it would be more restrictive if we forced MulAssign in all cases, but that at least that would be consistent. What do you think?

[bluss]

I would love to enable these by default with rustc-version. Not sure if it's proportionate to compile that crate too and call rustc another time. I think it's relatively lightweight, but still.

It's unfortunate to have to choose in that manner between Add and AddAssign, but I'll happily use AddAssign when it's possible.

[cuviper]

Hmm, sorry this lingered. We really should do a server bump already and be done with it...

Maybe I should open a devel or next branch to work on that? With some basic expectations, like we're not rewriting the world, just making incremental changes we couldn't do without breaking API.

[bluss]

I was just closing all kinds of PRs in different projects that had been inactive. In general I think num must develop and evolve from version to version instead of being stagnant. Don't you think it would be permissible with some version friction in exchange for a library that gets better from version to version?

[cuviper]

We need to figure something out, yes. The reason I'm so conservative with it is that num was already so widely used at the time we took maintainership that I don't want to rock that boat. With the traits especially, these are things that are more likely to show up in the public interface of other crates, which makes version bumps more painful than simple internal dependencies.

Even now num is # 12 most downloaded on crates.io, and for a little fresher info after we split crates, num-traits is # 27. It's hard to make a clean ranking though -- it would be most useful to limit all crates to a time period for comparing most downloaded. Anyway, I don't mention these numbers to brag, merely to gauge the potentially broad reach of breaking changes.

We should evolve, but I want to minimize the actual churn. I hope that in most cases, folks will be able to just bump their dependency and carry on.