Allow direct conversions from f32 to RealField or ComplexField #55
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #54. This should help avoid surprises for users of crates like nalgebra when, for instance, casting
f32
matrices toRealField
matrices.One concern that is likely to block a merge of this PR is that anyone who implements
RealField
andComplexField
would need to implement an additional trait (SupersetOf<f32>
). Given that bothf32
andf64
have are in frequent use, this might be better long-term, but it would require a major version bump.I'm not familiar enough with Rust to know whether it's possible to handle this in a backwards-compatible way. As far as I can tell, based on rust-lang/rust#31844, it's not possible to create a default implementation. If I were to write the following
it would not compile because it conflicts with the more specific macro in
subset.rs
that effectively runimpl SubsetOf<f32> for f32
andimpl SubsetOf<32> for f64
.The right answer might be to keep serde the way it is, although it may be good to document against potential pitfalls that could create. For instance, it hampers nalgebra's
cast
method for anyone trying to cast anf32
matrix to aRealField
matrix. However, ifRealField
is mostly meant to be used internally by dimforge crates, this might be a non-issue, in which case this PR should almost definitely be closed without merging.