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

min, max, minmax should use isless, not < #23094

Closed
StefanKarpinski opened this issue Aug 2, 2017 · 2 comments · Fixed by #23155
Closed

min, max, minmax should use isless, not < #23094

StefanKarpinski opened this issue Aug 2, 2017 · 2 comments · Fixed by #23155
Labels
breaking This change will break code
Milestone
0.7

Comments

@StefanKarpinski
Copy link
Member

See https://discourse.julialang.org/t/max-x-t-y-t-and-min-must-be-commutative/5165.
@StefanKarpinski StefanKarpinski added this to the 1.0 milestone Aug 2, 2017
@StefanKarpinski StefanKarpinski added the breaking This change will break code label Aug 2, 2017
@StefanKarpinski
Copy link
Member Author

StefanKarpinski commented Aug 2, 2017 •
edited
Loading

Very slightly technically breaking. See also #14621.

@KlausC
Copy link
Contributor

KlausC commented Aug 2, 2017

I saw the findmin and findmax are implemented using a1 < m || m!=m , which is targets the Float64 idiosyncrasies. In line with the proposed changes in min and max, the < should be replaced by isless for non-numeric types. Distinction by eltype(itr) required?

@KlausC KlausC mentioned this issue Aug 6, 2017
Merged
JeffBezanson pushed a commit that referenced this issue Aug 9, 2017
@KlausC @JeffBezanson
min, max, minmax use isless for comparison (fix #23094) (#23155)
5f582ae
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
breaking This change will break code
Projects
None yet
Milestone
0.7
Development

Successfully merging a pull request may close this issue.

min, max, minmax use isless for comparison (fix #23094)
2 participants
@StefanKarpinski @KlausC