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.

Sign up for GitHub

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

Jump to bottom

Support the same conversions from integer types as f64 and f32. #135

Merged
merged 2 commits into from
Aug 17, 2023
Merged

Support the same conversions from integer types as f64 and f32. #135

merged 2 commits into from
Aug 17, 2023

Conversation

blp
Copy link
Contributor

@blp blp commented Aug 17, 2023

The standard f64 and f32 types support conversion from integer types for which the conversion will not lose precision, as well as from bool. This commit adds that same support to OrderedFloat.

@blp
Support the same conversions from integer types as f64 and f32.
565c9c8 
The standard `f64` and `f32` types support conversion from integer types
for which the conversion will not lose precision, as well as from `bool`.
This commit adds that same support to `OrderedFloat`.
@blp blp mentioned this pull request Aug 17, 2023
Open
2 tasks
@blp
Copy link
Contributor Author

blp commented Aug 17, 2023

I see that the build failed because From<bool> for f64 and f32 is only supported in 1.68.0 and later. I can think of the following ways to fix that:

  • Bump the minimum Rust version to 1.68.
  • Define these conversion for OrderedFloat only in 1.68 and later.
  • Implement the conversions in OrderedFloat so that they don't try to use the underlying conversions for f64 and f32 but just implement them themselves.

Any preference?

@mbrubeck
Copy link
Collaborator

Implement the conversions in OrderedFloat so that they don't try to use the underlying conversions for f64 and f32 but just implement them themselves.

Let’s use this option.

@blp
Support bool to OrderedFloat conversions for Rust before 1.68.0.
a618c20 
The Rust standard library only implemented `From<bool>` for `f32` and `f64`
starting from version 1.68.0, so to support earlier versions this commit
implements those conversions by hand.
@blp
Copy link
Contributor Author

blp commented Aug 17, 2023

Implement the conversions in OrderedFloat so that they don't try to use the underlying conversions for f64 and f32 but just implement them themselves.

Let’s use this option.

Done ✔️

mbrubeck
mbrubeck approved these changes Aug 17, 2023
View reviewed changes
Copy link
Collaborator

@mbrubeck mbrubeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@mbrubeck mbrubeck merged commit d7768e7 into reem:master Aug 17, 2023
@blp blp deleted the from-integers branch August 17, 2023 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbrubeck mbrubeck mbrubeck approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@blp @mbrubeck