-
Notifications
You must be signed in to change notification settings - Fork 71
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
Prepare 34.0.0 Release #550
Conversation
@jdye64 fyi |
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
@@ -36,7 +36,7 @@ use std::sync::Arc; | |||
/// The actual execution of a plan runs natively on Rust and Arrow on a multi-threaded environment. | |||
#[pyclass(name = "DataFrame", module = "datafusion", subclass)] | |||
#[derive(Clone)] | |||
pub(crate) struct PyDataFrame { | |||
pub struct PyDataFrame { |
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.
This is also due to 34.0.0?
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 don't think directly. I will say having lots of these "(crate)" level visibility of structs makes using ADP as a library difficult since they are not visible outside the crate. I had started removing them awhile back and think Andy was just removing another one of them. I should probably just make a PR to remove them all, where it makes sense of course
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 removed the (crate)
restriction to work around many warnings like the following. I wonder if this was caused by a recent Rust version upgrade rather than anything related to 34.0.0.
warning: type `PyDataFrame` is more private than the item `PySessionContext::sql`
--> src/context.rs:282:5
|
282 | pub fn sql(&mut self, query: &str, py: Python) -> PyResult<PyDataFrame> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ method `PySessionContext::sql` is reachable at visibility `pub`
|
note: but type `PyDataFrame` is only usable at visibility `pub(crate)`
--> src/dataframe.rs:39:1
|
39 | pub(crate) struct PyDataFrame {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: `#[warn(private_interfaces)]` on by default
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.
There is a reference to private_interfaces
in the release notes for Rust 1.74.0, which was released last month.
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 see. Thanks for the reference.
Which issue does this PR close?
N/A
Rationale for this change
Prepare for next release
What changes are included in this PR?
Are there any user-facing changes?