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.

Sign up for GitHub

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

Jump to bottom

GH-15947: fixed skipped_column error in Python #16164

Open
wants to merge 1 commit into
base: rel-3.46.0
Choose a base branch
from
Open

GH-15947: fixed skipped_column error in Python #16164

wants to merge 1 commit into from
+56 −24

Conversation

wendycwong
Copy link
Contributor

@wendycwong wendycwong commented Apr 18, 2024 •
edited

issue: #15947

The problem here is when called with h2o.H2OFrame, we did not take into account of skipped columns when trying to figure out the final column counts.

Fixed the bug and added Python test from Seb.
Fixed the bug in R and added R test.

@wendycwong wendycwong added the do not merge For PRs that are not supposed to be merged label Apr 26, 2024
sebhrusen
sebhrusen reviewed May 13, 2024
View reviewed changes
Copy link
Contributor

@sebhrusen sebhrusen left a comment •
edited

Choose a reason for hiding this comment

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

thanks for this fix @wendycwong, but I must admit that I still don't understand why the old check is still in place.

h2o-py/tests/testdir_apis/Data_Manipulation/pyunit_h2oH2OFrame.py Outdated Show resolved Hide resolved
h2o-py/h2o/h2o.py Outdated Show resolved Hide resolved
h2o-py/h2o/h2o.py Outdated Show resolved Hide resolved
@wendycwong wendycwong removed the do not merge For PRs that are not supposed to be merged label May 21, 2024
@wendycwong wendycwong requested a review from sebhrusen May 21, 2024 21:08
maurever
maurever approved these changes May 22, 2024
View reviewed changes
Copy link
Contributor

@maurever maurever left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @wendycwong .

GH-15947: fixed column length discrepancies when skipped_columns are …
da7946f 
…specified when calling h2o.H2OFrame.

GH-15947: fixed skipped columns for normal import_file path.
GH-15947: added same skipped column capability when transforming R data frames to h2O data frames.
GH-15947: add parameter to avoid R cmd test failure.
GH-15947: Incorporate Seb code review comments.
@wendycwong wendycwong force-pushed the wendy_gh_15947_skipped_columns_bug branch from 57b323a to da7946f Compare May 23, 2024 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maurever maurever maurever approved these changes

@sebhrusen sebhrusen Awaiting requested review from sebhrusen

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@wendycwong @maurever @sebhrusen