Skip to content
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

[FEAT] connect: createDataFrame (WIP) help needed #3376

Closed
wants to merge 1 commit into from

Conversation

andrewgazelka
Copy link
Member

No description provided.

Copy link
Member Author

andrewgazelka commented Nov 21, 2024

@andrewgazelka andrewgazelka marked this pull request as ready for review November 21, 2024 00:56
.map(|name| daft_dsl::col(name))
.collect();

let plan = plan.with_columns(column_names)?
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing semicolon at the end of plan.with_columns(column_names)?. This will cause a compilation error.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

Comment on lines +3 to +10
pub fn local_relation(
local_relation: spark_connect::LocalRelation,
) -> eyre::Result<LogicalPlanBuilder> {
let spark_connect::LocalRelation {
data,
schema,
} = local_relation;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function needs to be completed - it currently lacks a return value and implementation. The function should process the data and schema parameters to construct and return a LogicalPlanBuilder. Consider adding error handling for invalid data or schema formats.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

Comment on lines +1 to +3
use eyre::{bail, WrapErr};
use daft_logical_plan::LogicalPlanBuilder;
use crate::translation::to_logical_plan;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The daft_dsl crate needs to be imported to support the col() function call on line 20. Consider adding use daft_dsl; to the imports section.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

@andrewgazelka andrewgazelka force-pushed the andrew/connect-binary-operators branch from 844a0b1 to dc3c34e Compare November 21, 2024 01:44
@andrewgazelka andrewgazelka force-pushed the andrew/connect-createDf branch from aeebe17 to 5fa8e8f Compare November 21, 2024 01:45
@andrewgazelka andrewgazelka force-pushed the andrew/connect-binary-operators branch from dc3c34e to 693574c Compare November 21, 2024 01:49
Copy link

graphite-app bot commented Nov 21, 2024

Graphite Automations

"Warn authors when publishing large PRs" took an action on this PR • (11/21/24)

1 teammate was notified to this PR based on Andrew Gazelka's automation.

@andrewgazelka andrewgazelka force-pushed the andrew/connect-createDf branch from 5fa8e8f to 427db3c Compare November 21, 2024 01:50
@andrewgazelka andrewgazelka force-pushed the andrew/connect-binary-operators branch from 693574c to c7f60fd Compare November 21, 2024 03:22
@andrewgazelka andrewgazelka force-pushed the andrew/connect-createDf branch from 427db3c to ef7a0bb Compare November 21, 2024 03:22
@andrewgazelka andrewgazelka force-pushed the andrew/connect-binary-operators branch from c7f60fd to 3a8c076 Compare November 21, 2024 04:19
@andrewgazelka andrewgazelka force-pushed the andrew/connect-createDf branch from ef7a0bb to f88c39c Compare November 21, 2024 04:19
@andrewgazelka andrewgazelka force-pushed the andrew/connect-binary-operators branch from 3a8c076 to 4d50b1d Compare November 21, 2024 04:21
@andrewgazelka andrewgazelka force-pushed the andrew/connect-binary-operators branch 2 times, most recently from 8f7a067 to 54468cc Compare November 21, 2024 05:16
@andrewgazelka andrewgazelka force-pushed the andrew/connect-createDf branch from f88c39c to b3ab393 Compare November 21, 2024 05:18
@andrewgazelka andrewgazelka force-pushed the andrew/connect-binary-operators branch from 54468cc to 28173ab Compare November 21, 2024 16:57
@andrewgazelka andrewgazelka force-pushed the andrew/connect-createDf branch from b3ab393 to 411958a Compare November 21, 2024 16:57
@andrewgazelka andrewgazelka mentioned this pull request Nov 21, 2024
1 task
@andrewgazelka andrewgazelka force-pushed the andrew/connect-binary-operators branch from 28173ab to cdca2e2 Compare November 21, 2024 18:43
@andrewgazelka andrewgazelka force-pushed the andrew/connect-createDf branch from 411958a to 6ff164c Compare November 21, 2024 18:43
@andrewgazelka andrewgazelka force-pushed the andrew/connect-binary-operators branch from cdca2e2 to fbc5c6e Compare November 27, 2024 07:42
@andrewgazelka andrewgazelka force-pushed the andrew/connect-createDf branch from 6ff164c to 9457c8e Compare November 27, 2024 07:42
@andrewgazelka andrewgazelka marked this pull request as draft November 27, 2024 21:04
@andrewgazelka andrewgazelka force-pushed the andrew/connect-binary-operators branch from fbc5c6e to a2e0beb Compare December 4, 2024 02:09
@andrewgazelka andrewgazelka force-pushed the andrew/connect-createDf branch from 9457c8e to ffc5efc Compare December 4, 2024 02:09
@andrewgazelka andrewgazelka force-pushed the andrew/connect-binary-operators branch 2 times, most recently from b9a7f70 to 3ee5757 Compare December 4, 2024 02:33
@andrewgazelka andrewgazelka force-pushed the andrew/connect-createDf branch 2 times, most recently from 7f6b1e3 to 464f6f5 Compare December 4, 2024 02:44
Base automatically changed from andrew/connect-binary-operators to main December 4, 2024 03:01
@github-actions github-actions bot added the enhancement New feature or request label Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant