-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
make vector cmp match axes
#59500
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
make vector cmp match axes
#59500
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
very tangential, but it's a shame to have to use exceptions here. partial orders seem like the perfect use case for a |
|
@nanosoldier |
|
The package evaluation job you requested has completed - possible new issues were detected. Report summary❗ Packages that crashed2 packages crashed on the previous version too. ✖ Packages that failed21 packages failed only on the current version.
1339 packages failed on the previous version too. ✔ Packages that passed tests13 packages passed tests only on the current version.
5214 packages passed tests on the previous version too. ~ Packages that at least loaded9 packages successfully loaded only on the current version.
3196 packages successfully loaded on the previous version too. ➖ Packages that were skipped altogether22 packages were skipped only on the current version.
1132 packages were skipped on the previous version too. |
|
@nanosoldier |
|
The package evaluation job you requested has completed - possible new issues were detected. Report summary✖ Packages that failed2 packages failed only on the current version.
2 packages failed on the previous version too. ✔ Packages that passed tests1 packages passed tests only on the current version.
4 packages passed tests on the previous version too. ~ Packages that at least loaded1 packages successfully loaded on the previous version too. |
|
bump |
|
Looks reasonable to me. Should this be mentioned in the docs somewhere? |
|
arguably comparing indices first is "already" lexicographic, and it's just a question of how do you align / zip the axes together, but good idea to spell that out explicitly |
jishnub
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable to me.
|
CI failure is #59650 seeing two approvals: merging |
closes JuliaLang#48916 as suggested by @oscardssmith > I believe the more consistent behavior here would be for isless to error when comparing two Arrays a and b where eachindex(a,b)∉(eachindex(a), eachindex(b)) (similarly to how it works with Complex numbers or other collections like Dict that don't have an ordering.
closes #48916 as suggested by @oscardssmith