Skip to content

TST: Test Loc to set Multiple Items to multiple new columns #42665

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
Merged
merged 9 commits into from
Jul 28, 2021
Merged

TST: Test Loc to set Multiple Items to multiple new columns #42665

merged 9 commits into from
Jul 28, 2021

Conversation

GYvan
Copy link
Contributor

@GYvan GYvan commented Jul 22, 2021 •
edited
Loading

@pep8speaks
Copy link

pep8speaks commented Jul 22, 2021 •
edited
Loading

Hello @GYvan! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-07-27 15:01:12 UTC

@simonjayhawkins simonjayhawkins added Indexing Related to indexing on series/frames, not to indexes themselves Testing pandas testing functions or related to the test suite labels Jul 22, 2021
@simonjayhawkins
Copy link
Member

Thanks @GYvan for the PR. There are some lint failures and another git commit.

see https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#working-with-the-code

@GYvan
Copy link
Contributor Author

GYvan commented Jul 22, 2021

Thanks @GYvan for the PR. There are some lint failures and another git commit.

see https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#working-with-the-code

Thanks, I will look through them

@GYvan GYvan changed the title Testing_setMultipleItems_to_multiple_newColumns TST: Test Loc to set Multiple Items to multiple new columns Jul 23, 2021
@GYvan
Copy link
Contributor Author

GYvan commented Jul 26, 2021

Thanks @GYvan for the PR. There are some lint failures and another git commit.

see https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#working-with-the-code

@GYvan GYvan closed this Jul 26, 2021
@GYvan
Copy link
Contributor Author

GYvan commented Jul 26, 2021

Thanks @GYvan for the PR. There are some lint failures and another git commit.
see https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#working-with-the-code

Thanks @GYvan for the PR. There are some lint failures and another git commit.

see https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#working-with-the-code

@GYvan GYvan reopened this Jul 26, 2021
@GYvan
Copy link
Contributor Author

GYvan commented Jul 26, 2021

Hi @simonjayhawkins. I have tried to address the lint failures and reverted the unrelate changes. Can you please let me know if anything else needs changes?

@jbrockmendel
Copy link
Member

cc @phofl

jreback
jreback requested changes Jul 26, 2021
View reviewed changes
pandas/tests/indexing/test_loc.py Outdated
expected.loc[1, "b"] = 6
expected.loc[1, "c"] = 7

tm.assert_frame_equal(result, expected, check_dtype=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

does this fail w/o the check_dtype?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, as it should. The expected should have dtype object while df has dtype float in these columns

phofl
phofl requested changes Jul 26, 2021
View reviewed changes
pandas/tests/indexing/test_loc.py Outdated
# GH 25594
df = DataFrame(index=[1, 2], columns=["a"])
df.loc[1, ["b", "c"]] = [6, 7]
result = df.copy()
Copy link
Member

Choose a reason for hiding this comment

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

No result needed, just use df

pandas/tests/indexing/test_loc.py Outdated
df.loc[1, ["b", "c"]] = [6, 7]
result = df.copy()

expected = DataFrame(index=[1, 2], columns=["a", "b", "c"])
Copy link
Member

Choose a reason for hiding this comment

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

Please define expected without the use of loc. You can show the result of the sequential operations too, but please use another check for this

@GYvan GYvan requested review from phofl and jreback July 27, 2021 11:25
phofl
phofl reviewed Jul 27, 2021
View reviewed changes
pandas/tests/indexing/test_loc.py Outdated

tm.assert_frame_equal(result, expected, check_dtype=False)
tm.assert_frame_equal(df, expected, check_dtype=False)
Copy link
Member

Choose a reason for hiding this comment

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

Please remove the check_dtype=False parameter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@phofl I tried the remove the "check_dtype=False" parameter put the test is not passing. This is the error I'm getting: "- AssertionError: Attributes of DataFrame.iloc[:, 0] (column name="a") are different". I think the dtype of df and expected are different

Copy link
Member

Choose a reason for hiding this comment

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

Then you have to define expected accordingly

@GYvan GYvan requested a review from phofl July 27, 2021 15:08
@jreback
Copy link
Contributor

jreback commented Jul 28, 2021

lgtm. @phofl merge when ready

@phofl phofl merged commit 24305ab into pandas-dev:master Jul 28, 2021
@phofl
Copy link
Member

phofl commented Jul 28, 2021

Thanks @GYvan

@GYvan GYvan deleted the my_new_Branch branch July 28, 2021 21:09
CGe0516 pushed a commit to CGe0516/pandas that referenced this pull request Jul 29, 2021
@GYvan @CGe0516
TST: Test Loc to set Multiple Items to multiple new columns (pandas-d…
dd16b8f 
…ev#42665)

* Modified ecosystem.rst to include ibis

* created a test for issue pandas-dev#25594

* Test for issue pandas-dev#25594

* reverted the changes

* Test Loc to set Multiple Items to multiple new columns - Changes Made

* Test Loc to set Multiple Items to multiple new columns - Changes made and linting addresssed

* TST: Test Loc to set Multiple Items to multiple new columns - Changes Made and pre-commit

* TST: test for issue pandas-dev#25594 - Changes made

* removed the check_dtype
feefladder pushed a commit to feefladder/pandas that referenced this pull request Sep 7, 2021
@GYvan @feefladder
TST: Test Loc to set Multiple Items to multiple new columns (pandas-d…
da562aa 
…ev#42665)

* Modified ecosystem.rst to include ibis

* created a test for issue pandas-dev#25594

* Test for issue pandas-dev#25594

* reverted the changes

* Test Loc to set Multiple Items to multiple new columns - Changes Made

* Test Loc to set Multiple Items to multiple new columns - Changes made and linting addresssed

* TST: Test Loc to set Multiple Items to multiple new columns - Changes Made and pre-commit

* TST: test for issue pandas-dev#25594 - Changes made

* removed the check_dtype
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback approved these changes

@phofl phofl Awaiting requested review from phofl

Assignees
No one assigned
Labels
Indexing Related to indexing on series/frames, not to indexes themselves Testing pandas testing functions or related to the test suite
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Adding multiple columns with loc fails
6 participants
@GYvan @pep8speaks @simonjayhawkins @jbrockmendel @jreback @phofl