-
Notifications
You must be signed in to change notification settings - Fork 922
Publically re-export rand
with test_utils
#7128
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
Conversation
I don't feel strongly about this and if it is not wanted we can just close this PR |
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.
I am largely indifferent to this
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.
It may just be better to avoid using the arrow bench_utils at all.
+1
But it's also a good idea to merge this.
What do we think about merging this and marking all of bench_utils as deprecated (pointing out that external users should just copy the code into their repo if they want to use them) 🤔 |
Filed a ticket to track making the API deprecated |
This seems like more trouble than it is worth and I don't have time to pursue it, so closing for now |
Which issue does this PR close?
55.0.0
(Apr 2025) #7084Rationale for this change
As we found out here
rand
appears in some public APIs and thus it should be available to use with just arrowFor example, in DataFusion we have
https://github.com/apache/datafusion/blob/dfc4ba31ed26fbc84eebb0c39b96ebd8f2c11267/datafusion/functions-aggregate/benches/array_agg.rs#L51-L58
(namely randomly generating a LIstArray using the
create_primitive_array
function from bench_utils)Being able to have datafusion's benchmark use the same rand / distribution would let us use both versions
It may just be better to avoid using the arrow bench_utils at all.
What changes are included in this PR?
Propose publically re-exporting
rand
and using that re-export in the public APIsAre there any user-facing changes?
New public export when
test_utils
feature is enabled