Skip to content
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

relaxing the trait bounds on Complex::Add and friends #124

Open
sarah-quinones opened this issue Mar 11, 2024 · 1 comment
Open

relaxing the trait bounds on Complex::Add and friends #124

sarah-quinones opened this issue Mar 11, 2024 · 1 comment

Comments

@sarah-quinones
Copy link

at the moment, all the core arithmetic operations for Complex require the Num bound to be satisfied
https://docs.rs/num-complex/latest/num_complex/struct.Complex.html#impl-Add-for-Complex%3CT%3E

this is stronger than it needs to be, and implementing that can be difficult for new numerical types like qd::Double<f64>, due to the Num::from_str_radix being nontrivial to implement

would it be acceptable to relax the constraint to Add<Output = Self> for addition, Sub<Output = Self> for subtraction, and Mul<Output = Self> + Add<Output = Self> + Sub<Output = Self> for multiplication?

@cuviper
Copy link
Member

cuviper commented Mar 11, 2024

I think we can do that safely, without any breaking change even, but I do worry about relaxing too far. Relaxing in one direction becomes a limitation in the other, that it leaves little room for making new implementation choices. Plus, a long list of constraints is more of a burden for both the reader and the maintainer.

Maybe we should just add a local trait like:

pub trait ComplexNum: PartialEq + Zero + One + NumOps {}

That is everything from Num except for from_str_radix. The only place we actually use that method is when implementing it for Complex itself. We might also want to split out NumOps and drop Rem, since that's pretty weird for Complex, and we only use it for our own Rem and for div_trunc.

I think it's also fine if a type only implements Num::from_str_radix for "easy" radices at first, or even an unconditional Err if it's really too hard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants