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

Added copysign fixes #152 #155

Closed
wants to merge 1 commit into from
Closed

Added copysign fixes #152 #155

wants to merge 1 commit into from

Conversation

peartt
Copy link

@peartt peartt commented Feb 13, 2020

No description provided.

@cuviper cuviper changed the title Added copysign fixes https://github.com/rust-num/num-traits/issues/152 Added copysign fixes #152 Mar 6, 2020
@cuviper
Copy link
Member

cuviper commented Mar 6, 2020

Please look at the failed CI results, and let me know if you need help...

Comment on lines +1832 to +1834
if self.is_nan() {
sign / Self::zero()
} else if self.signum() == sign.signum() {
Copy link
Contributor

Choose a reason for hiding this comment

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

is_sign_negative is better than signum here as it works on NaNs as well; signum itself is implemented using is_sign_negative in the trait.

Then the special case for NaN can be removed as both is_sign_negative and neg work well on NaNs. (Currently the NaN special case is buggy anyway and returns infinity rather than NaN.)

@XAMPPRocky
Copy link
Contributor

@peartt Hello, are you still interested in continuing this PR? 🙂 As I would be interested in seeing this merged so we can use it in a project, and would be willing to continue the work.

@XAMPPRocky XAMPPRocky mentioned this pull request Mar 17, 2021
@bors bors bot closed this in #207 May 3, 2022
@bors bors bot closed this in edb4821 May 3, 2022
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 this pull request may close these issues.

4 participants