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

Fix equality for one-element ranges #36074

Merged
merged 1 commit into from
Jun 4, 2020

Conversation

sostock
Copy link
Contributor

@sostock sostock commented May 29, 2020

#32348 made zero-element ranges compare equal, but some types of one-element ranges do still not compare equal if their step is different (cf. #29421 (comment), thanks to @ti-s for pointing this out). This PR fixes that.

As discussed in #29421, this should be backported to 1.5. Should this be mentioned in the NEWS.md/HISTORY.md item for #32348?

@mbauman mbauman added the triage This should be discussed on a triage call label May 29, 2020
@rfourquet rfourquet added this to the 1.5 milestone May 30, 2020
@KristofferC
Copy link
Member

@mbauman, what is there to triage here?

@mbauman
Copy link
Member

mbauman commented Jun 3, 2020

It's just a pretty big behavior change for a backport. I personally am fully in favor of it getting backported — and maybe it's not controversial — but I thought it'd be worth talking about. Perhaps simply a PkgEval is enough:

@nanosoldier runtests(ALL, vs = ":master")

@nanosoldier
Copy link
Collaborator

Your package evaluation job has completed - possible new issues were detected. A full report can be found here. cc @maleadt

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Jun 4, 2020

It seems like a very buggy/wonky situation to have changed the zero-length behavior and not the one-length behavior. Definitely in favor of backporting this as a bug fix.

@mbauman
Copy link
Member

mbauman commented Jun 4, 2020

PkgEval results seem to be pretty good (and just noisy failures), but just out of curiousity:

@nanosoldier runtests(["BilevelJuMP", "CodeTransformation", "DelayDiffEq", "Juniper", "MatrixProfile", "Pajarito", "Reactive", "TensorToolbox", "WinRPM"], vs = ":master")

@JeffBezanson
Copy link
Member

Agreed we should backport it.

@JeffBezanson JeffBezanson removed the triage This should be discussed on a triage call label Jun 4, 2020
@mbauman mbauman merged commit 468555d into JuliaLang:master Jun 4, 2020
@nanosoldier
Copy link
Collaborator

Your package evaluation job has completed - no new issues were detected. A full report can be found here. cc @maleadt

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.

7 participants