-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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 sorting for iterables that define copymutable #52086
Conversation
`copymutable` is only defined to return an array for abstract arrays, but that is only what this method is never called with. For other types, it has a default of `collect`, but can be changed by other types (such as AbstractSet) to do something different. Refs #46104
Since we only support sorting abstract arrays with This would be inconvenient for iterable and sortable non-abstractarray types that bring their own
|
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.
LGTM, though we should probably wait to see if triage decides to revert #46104 before we merge this.
We don't need triage to approve merging a bugfix, even if there may be later changes to the API |
Two chagnes wrapped into one `Base.copymutable` => `Base.copymutable` & `collect` and `Base.copymutable` => `similar` & words. Followup for #52086 and #46104; also fixes #51932 (though we still may want to make `copymutable` public at some point) --------- Co-authored-by: Jameson Nash <vtjnash@gmail.com>
(cherry picked from commit f99e6bf)
Two chagnes wrapped into one `Base.copymutable` => `Base.copymutable` & `collect` and `Base.copymutable` => `similar` & words. Followup for #52086 and #46104; also fixes #51932 (though we still may want to make `copymutable` public at some point) --------- Co-authored-by: Jameson Nash <vtjnash@gmail.com> (cherry picked from commit 42c088b)
This reverts commit f99e6bf.
This reverts commit f99e6bf.
copymutable
is only defined to return an array for abstract arrays, but that is only what this method is never called with. For other types, it has a default ofcollect
(which means the default behavior appears to give an array), but can be changed by other types (such as AbstractSet) to do something different.Correction to #46104