-
Notifications
You must be signed in to change notification settings - Fork 867
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
Sort binary #569
Sort binary #569
Conversation
Codecov Report
@@ Coverage Diff @@
## master #569 +/- ##
==========================================
+ Coverage 82.41% 82.49% +0.07%
==========================================
Files 167 167
Lines 46257 46465 +208
==========================================
+ Hits 38125 38331 +206
- Misses 8132 8134 +2
Continue to review full report at Codecov.
|
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.
Thank you @waynexia
I am not super familiar with the internals of list generic binary array, but I reviewed the tests you have added (thank you!) and they seem to demonstrate the correct behavior.
@nevi-me or @jorgecarleitao might you have some time to review this PR?
Signed-off-by: Ruihang Xia <[email protected]>
Signed-off-by: Ruihang Xia <[email protected]>
Signed-off-by: Ruihang Xia <[email protected]>
Signed-off-by: Ruihang Xia <[email protected]>
Signed-off-by: Ruihang Xia <[email protected]>
Signed-off-by: Ruihang Xia <[email protected]>
Signed-off-by: Ruihang Xia <[email protected]>
Unless I hear anything else I plan to merge this later today or tomorrow |
Thanks again @waynexia 🚀 |
Appreciate it @alamb! |
BTW @waynexia I expect this change to be available in arrow 5.1.0 (scheduled for release at the end of this week or early next week) |
* impl sort fixed binary array Signed-off-by: Ruihang Xia <[email protected]> * remove array builder util, add test cases Signed-off-by: Ruihang Xia <[email protected]> * impl sort for generic binary array Signed-off-by: Ruihang Xia <[email protected]> * tidy Signed-off-by: Ruihang Xia <[email protected]> * run clippy Signed-off-by: Ruihang Xia <[email protected]> * test: fixed binary with different prefix Signed-off-by: Ruihang Xia <[email protected]> * rebase master Signed-off-by: Ruihang Xia <[email protected]>
Sure! I'd like to help. It seems that changes in #552 make the cherry-pick PR's CI failed. I think #552 should be cherry-picked first. However, #552 changes the default behavior (from stable sort to unstable version). So if ship #552 in a minor release is not expected, I can submit a patch to make #621 pass CI. |
Got it, I opened #624 😃 |
* Sort binary (#569) * impl sort fixed binary array Signed-off-by: Ruihang Xia <[email protected]> * remove array builder util, add test cases Signed-off-by: Ruihang Xia <[email protected]> * impl sort for generic binary array Signed-off-by: Ruihang Xia <[email protected]> * tidy Signed-off-by: Ruihang Xia <[email protected]> * run clippy Signed-off-by: Ruihang Xia <[email protected]> * test: fixed binary with different prefix Signed-off-by: Ruihang Xia <[email protected]> * rebase master Signed-off-by: Ruihang Xia <[email protected]> * Fix compile error in cherry pick 2113e2b (#624) Signed-off-by: Ruihang Xia <[email protected]> Co-authored-by: Ruihang Xia <[email protected]>
Which issue does this PR close?
Closes #568.
What changes are included in this PR?