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

ARROW-9495: [C++] Equality assertions don't handle Inf / -Inf properly #7826

Closed
wants to merge 1 commit into from

Conversation

liyafan82
Copy link
Contributor

@github-actions
Copy link

// Infinity in a valid entry
ArrayFromVector<TYPE>(type, {false, true}, {0.5, infinity}, &a);
ArrayFromVector<TYPE>(type, {false, true}, {0.5, infinity}, &b);
std::cout << a->ToString() << std::endl;
Copy link
Member

Choose a reason for hiding this comment

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

nit: we could remove this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed. Thanks for your kind reminder.

@kiszk
Copy link
Member

kiszk commented Aug 12, 2020

Looks good to me

@liyafan82
Copy link
Contributor Author

@kiszk Thanks a lot for your feedback.
@pitrou could you please give some comments?

left, right, [epsilon](T x, T y) -> bool { return fabs(x - y) <= epsilon; });
return BaseFloatingEquals<ArrowType>(left, right, [epsilon](T x, T y) -> bool {
return (fabs(x - y) <= epsilon) || (x == y);
});
Copy link
Member

Choose a reason for hiding this comment

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

You must also change the nans_equal path above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Revised accordingly. Thanks a lot for your kind reminder.

Copy link
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

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

+1. Thank you @liyafan82 !

@pitrou pitrou closed this in c4f8436 Aug 17, 2020
emkornfield pushed a commit to emkornfield/arrow that referenced this pull request Sep 8, 2020
See https://issues.apache.org/jira/browse/ARROW-9495

Closes apache#7826 from liyafan82/fly_0724_eq

Authored-by: liyafan82 <fan_li_ya@foxmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
emkornfield pushed a commit to emkornfield/arrow that referenced this pull request Oct 16, 2020
See https://issues.apache.org/jira/browse/ARROW-9495

Closes apache#7826 from liyafan82/fly_0724_eq

Authored-by: liyafan82 <fan_li_ya@foxmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
GeorgeAp pushed a commit to sirensolutions/arrow that referenced this pull request Jun 7, 2021
See https://issues.apache.org/jira/browse/ARROW-9495

Closes apache#7826 from liyafan82/fly_0724_eq

Authored-by: liyafan82 <fan_li_ya@foxmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
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.

None yet

3 participants