kosiew commented on code in PR #24633:
URL: https://github.com/apache/datafusion/pull/24633#discussion_r3886196400
##########
datafusion/core/src/dataframe/mod.rs:
##########
@@ -2628,17 +2628,16 @@ impl DataFrame {
/// # Ok(())
/// # }
/// ```
- pub fn from_columns(columns: Vec<(&str, ArrayRef)>) -> Result<Self> {
- let fields = columns
- .iter()
- .map(|(name, array)| Field::new(*name, array.data_type().clone(),
true))
- .collect::<Vec<_>>();
-
- let arrays = columns
+ pub fn from_columns<'a, I>(columns: I) -> Result<Self>
Review Comment:
Thanks for updating this. One API compatibility concern still remains here.
This changes the released non-generic `from_columns(Vec<(&str, ArrayRef)>)`
signature to `from_columns<'a, I>`, so the SemVer/API-health issue is still
present. `cargo-semver-checks` will report
`method_requires_different_generic_type_params`, and downstream code that uses
this method as a non-generic function item can break.
Using parameter-position `impl IntoIterator<Item = (&str, ArrayRef)>` may
avoid that specific cargo-semver-checks diagnostic and would preserve normal
`Vec` call syntax, but it is still an implicit generic parameter, so it would
not be a strict compatibility fix either.
To preserve the existing public API, I think the safest option is to keep
`from_columns(Vec<...>)` and add the iterator or array-taking behavior under a
new method name. The old API could then be deprecated later according to policy
if desired.
If this signature change is intentional instead, it should be treated and
documented as a breaking API change under the API-health policy, including the
`api-change` label and upgrade guidance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
