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

SNOW-1348621 Fix bug and add support for DataFrame key in getitem #1445

Merged
merged 4 commits into from
Apr 29, 2024
Merged

SNOW-1348621 Fix bug and add support for DataFrame key in getitem #1445

merged 4 commits into from
Apr 29, 2024

Conversation

sfc-gh-vbudati
Copy link
Contributor

Please answer these questions before submitting your pull requests. Thanks!

  1. What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

    Fixes SNOW-1348621

  2. Fill out the following pre-review checklist:

    • I am adding a new automated test(s) to verify correctness of my new code
    • I am adding new logging messages
    • I am adding a new telemetry message
    • I am adding new credentials
    • I am adding a new dependency

  3. Please describe how your code solves the related issue.

    (boolean) DataFrame keys in __getitem__ were not correctly supported. I use where() to evaluate the result for df[DataFrame key].

@sfc-gh-vbudati sfc-gh-vbudati added NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md NO-PANDAS-CHANGELOG-UPDATES NO-PANDAS-CHANGEDOC-UPDATES This PR does not update Snowpark pandas docs labels Apr 26, 2024
@sfc-gh-vbudati sfc-gh-vbudati requested a review from a team as a code owner April 26, 2024 22:48
sfc-gh-helmeleegy
sfc-gh-helmeleegy reviewed Apr 26, 2024
View reviewed changes
tests/integ/modin/series/test_getitem.py Outdated

@sql_count_checker(query_count=1)
def test_series_getitem_lambda_series():
data = {"a": 1, "b": 2, "c": 3}
Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing about negative numbers.

@sfc-gh-helmeleegy
Copy link
Contributor

Looks good overall. Just a couple of nits.

sfc-gh-mvashishtha
sfc-gh-mvashishtha approved these changes Apr 26, 2024
View reviewed changes
Copy link
Contributor

@sfc-gh-mvashishtha sfc-gh-mvashishtha left a comment

Choose a reason for hiding this comment

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

great work! LGTM with a nit

tests/integ/modin/frame/test_getitem.py Outdated Show resolved Hide resolved
sfc-gh-azhan
sfc-gh-azhan approved these changes Apr 26, 2024
View reviewed changes
Copy link
Collaborator

@sfc-gh-azhan sfc-gh-azhan left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix!

sfc-gh-helmeleegy
sfc-gh-helmeleegy reviewed Apr 26, 2024
View reviewed changes
Copy link
Contributor

@sfc-gh-helmeleegy sfc-gh-helmeleegy left a comment

Choose a reason for hiding this comment

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

Looks great. Thank you!

@sfc-gh-vbudati sfc-gh-vbudati merged commit fdc3f09 into main Apr 29, 2024
26 checks passed
@sfc-gh-vbudati sfc-gh-vbudati deleted the vbudati/SNOW-1348621-df-getitem-bug branch April 29, 2024 16:54
@github-actions github-actions bot locked and limited conversation to collaborators Apr 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sfc-gh-joshi sfc-gh-joshi sfc-gh-joshi left review comments

@sfc-gh-nkrishna sfc-gh-nkrishna sfc-gh-nkrishna left review comments

@sfc-gh-helmeleegy sfc-gh-helmeleegy sfc-gh-helmeleegy approved these changes

@sfc-gh-mvashishtha sfc-gh-mvashishtha sfc-gh-mvashishtha approved these changes

@sfc-gh-azhan sfc-gh-azhan sfc-gh-azhan approved these changes

@sfc-gh-nkumar sfc-gh-nkumar Awaiting requested review from sfc-gh-nkumar sfc-gh-nkumar was automatically assigned from snowflakedb/snowpandas

@sfc-gh-evandenberg sfc-gh-evandenberg Awaiting requested review from sfc-gh-evandenberg sfc-gh-evandenberg was automatically assigned from snowflakedb/snowpandas

Assignees
No one assigned
Labels
NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md NO-PANDAS-CHANGEDOC-UPDATES This PR does not update Snowpark pandas docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sfc-gh-vbudati @sfc-gh-helmeleegy @sfc-gh-azhan @sfc-gh-nkrishna @sfc-gh-mvashishtha @sfc-gh-joshi