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

Why require T : Num for implementing Deserialize for Complex<T> #118

Closed
WalterSmuts opened this issue Dec 2, 2023 · 3 comments · Fixed by #119
Closed

Why require T : Num for implementing Deserialize for Complex<T> #118

WalterSmuts opened this issue Dec 2, 2023 · 3 comments · Fixed by #119

Comments

@WalterSmuts
Copy link
Contributor

WalterSmuts commented Dec 2, 2023

Question in the title: Why require T : Num for implementing Deserialize for Complex<T>?

Asking because I'm attempting to implement Serialize and Deserialize for a private struct. I've got a minimal example:

use num_complex::Complex;
use serde::Deserialize;
use serde::Serialize;

#[derive(Serialize, Deserialize)]
struct Foo<T: Clone>(Complex<T>);

producing:

    Checking serde_test v0.1.0 (/home/walter/Development/serde_test)
error[E0277]: the trait bound `T: num_traits::Num` is not satisfied
 --> src/lib.rs:6:22
  |
6 | struct Foo<T: Clone>(Complex<T>);
  |                      ^^^^^^^ the trait `num_traits::Num` is not implemented for `T`
  |
  = note: required for `Complex<T>` to implement `Deserialize<'_>`
help: consider further restricting this bound
  |
6 | struct Foo<T: Clone + num_traits::Num>(Complex<T>);
  |                     +++++++++++++++++

All probable types I'd implement is likely to implement Num anyway so that's not really the concern. The concern is that it would require me to add the trait bound and cause a major version change in my library (if I want to use the derive macros).

The num-complex crate does compile and tests succeed with the bound removed, so I'm a bit confused as to why the bound was added in the first place:

$ git diff
diff --git a/src/lib.rs b/src/lib.rs
index cb847b8..594f5b6 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1508,7 +1508,7 @@ where
 #[cfg(feature = "serde")]
 impl<'de, T> serde::Deserialize<'de> for Complex<T>
 where
-    T: serde::Deserialize<'de> + Num + Clone,
+    T: serde::Deserialize<'de> + Clone,
 {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where

and

$ cargo test
   Compiling num-complex v0.4.4 (/home/walter/Development/num-complex)
    Finished test [unoptimized + debuginfo] target(s) in 0.40s
     Running unittests src/lib.rs (target/debug/deps/num_complex-a63a10dd3428759a)

running 92 tests
test cast::test::test_as_primitive ... ok
test cast::test::test_from_primitive ... ok
test cast::test::test_num_cast ... ok
test complex_float::test::test_arg ... ok
test cast::test::test_to_primitive ... ok
test complex_float::test::test_exp2 ... ok
test complex_float::test::test_conj ... ok
test complex_float::test::test_is_finite ... ok
test complex_float::test::test_exp ... ok
test complex_float::test::test_is_infinite ... ok
test complex_float::test::test_is_nan ... ok
test complex_float::test::test_log2 ... ok
test complex_float::test::test_is_normal ... ok
test complex_float::test::test_powi ... ok
test complex_float::test::test_log10 ... ok
test complex_float::test::test_powz ... ok
test test::complex_arithmetic::test_add ... ok
test test::complex_arithmetic::test_div ... ok
test test::complex_arithmetic::test_mul ... ok
test test::complex_arithmetic::test_mul_add ... ok
test test::complex_arithmetic::test_mul_add_float ... ok
test test::complex_arithmetic::test_neg ... ok
test test::complex_arithmetic::test_rem ... ok
test test::complex_arithmetic::test_sub ... ok
test test::float::test_acos ... ok
test test::float::test_acosh ... ok
test test::float::test_arg ... ok
test test::float::test_asin ... ok
test test::float::test_asinh ... ok
test test::float::test_atan ... ok
test test::float::test_atanh ... ok
test test::float::test_cbrt ... ok
test test::float::test_cbrt_imag ... ok
test test::float::test_cis ... ok
test test::float::test_cbrt_real ... ok
test test::float::test_cos ... ok
test test::float::test_cosh ... ok
test test::float::test_exp ... ok
test test::float::test_exp2 ... ok
test test::float::test_exp2_log ... ok
test test::float::test_exp_ln ... ok
test test::float::test_hyperbolic_identites ... ok
test test::float::test_ln ... ok
test test::float::test_log ... ok
test test::float::test_log10 ... ok
test test::float::test_log2 ... ok
test test::float::test_norm ... ok
test test::float::test_polar_conv ... ok
test test::float::test_powc ... ok
test test::float::test_powf ... ok
test test::float::test_sin ... ok
test test::float::test_sinh ... ok
test test::float::test_some_expf_cases ... ok
test test::float::test_sqrt ... ok
test test::float::test_sqrt_imag ... ok
test test::float::test_sqrt_real ... ok
test test::float::test_tan ... ok
test test::float::test_tanh ... ok
test test::float::test_trig_identities ... ok
test test::float::test_trig_to_hyperbolic ... ok
test test::real_arithmetic::test_add ... ok
test test::real_arithmetic::test_div ... ok
test test::real_arithmetic::test_div_rem_gaussian ... ok
test test::real_arithmetic::test_mul ... ok
test test::real_arithmetic::test_rem ... ok
test test::real_arithmetic::test_sub ... ok
test test::test_conj ... ok
test test::test_const ... ok
test test::test_consts ... ok
test test::test_divide_by_zero_natural - should panic ... ok
test test::test_from_str_fail ... ok
test test::test_from_str ... ok
test test::test_from_str_radix ... ok
test test::test_from_str_radix_fail - should panic ... ok
test test::test_hash ... ok
test test::test_inv ... ok
test test::test_hashset ... ok
test test::test_inv_zero ... ok
test test::test_is_finite ... ok
test test::test_is_infinite ... ok
test test::test_is_nan ... ok
test test::test_is_nan_special_cases ... ok
test test::test_is_normal ... ok
test test::test_l1_norm ... ok
test test::test_one ... ok
test test::test_prod ... ok
test test::test_pow ... ok
test test::test_scale_unscale ... ok
test test::test_sum ... ok
test test::test_string_formatting ... ok
test test::test_to_string ... ok
test test::test_zero ... ok

test result: ok. 92 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests num-complex

running 3 tests
test src/lib.rs - Complex (line 70) ... ok
test src/lib.rs - Complex<T>::finv (line 548) ... ok
test src/lib.rs - Complex<T>::fdiv (line 576) ... ok

test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.11s
@cuviper
Copy link
Member

cuviper commented Dec 5, 2023

I think it's probably just because the implementation uses Complex::new, which (unnecessarily) required those bounds until #63. But even before that, we could have just built the struct directly, had anyone bothered.

Would you like to open a PR to relax this?

WalterSmuts added a commit to WalterSmuts/num-complex that referenced this issue Dec 5, 2023
@WalterSmuts
Copy link
Contributor Author

Would you like to open a PR to relax this?

Sure. Thanks for having a look.

@WalterSmuts
Copy link
Contributor Author

@cuviper can you have a look at #119

cuviper pushed a commit to WalterSmuts/num-complex that referenced this issue Feb 6, 2024
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

Successfully merging a pull request may close this issue.

2 participants