Skip to content

TST: Add test for setting item using python list (#19406) #37840

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 4 commits into from
Nov 15, 2020
Merged

TST: Add test for setting item using python list (#19406) #37840

merged 4 commits into from
Nov 15, 2020

Conversation

paxcodes
Copy link
Contributor

arw2019
arw2019 reviewed Nov 14, 2020
View reviewed changes
Copy link
Member

@arw2019 arw2019 left a comment

Choose a reason for hiding this comment

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

Thanks @paxcodes for the PR!

Some small comments, otherwise LGTM

ivanovmg
ivanovmg reviewed Nov 14, 2020
View reviewed changes
Copy link
Member

@ivanovmg ivanovmg left a comment

Choose a reason for hiding this comment

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

@paxcodes thank you for the PR!
Would you consider parametrizing the test with various null values?

@paxcodes paxcodes requested a review from phofl November 14, 2020 20:02
@phofl
Copy link
Member

phofl commented Nov 14, 2020

One nitpick from my side: you coud parametrize somethink like func with [list, np.array, Series] and define mask in the test through mask=func([True, False, True]) Would make parametrization shorter

@paxcodes
Copy link
Contributor Author

@phofl Ooh nice. Will do.

@paxcodes
Copy link
Contributor Author

@phofl There. Much more concise. ✨

@phofl
Copy link
Member

phofl commented Nov 14, 2020

lgtm

ivanovmg
ivanovmg approved these changes Nov 15, 2020
View reviewed changes
Copy link
Member

@ivanovmg ivanovmg left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good to me.

@MarcoGorelli MarcoGorelli added this to the 1.2 milestone Nov 15, 2020
@MarcoGorelli MarcoGorelli added the Testing pandas testing functions or related to the test suite label Nov 15, 2020
@MarcoGorelli
Copy link
Member

Thanks @paxcodes , congrats on your first PR to pandas! Do let us know if you want pointers for other issues if you'd like to tackle them too

@MarcoGorelli MarcoGorelli merged commit 5b98dc6 into pandas-dev:master Nov 15, 2020
@paxcodes
Copy link
Contributor Author

Thanks @MarcoGorelli and again, thanks for your time yesterday at the sprint!

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

@ivanovmg ivanovmg ivanovmg approved these changes

@arw2019 arw2019 Awaiting requested review from arw2019

@phofl phofl Awaiting requested review from phofl

Assignees
No one assigned
Labels
Testing pandas testing functions or related to the test suite
Projects
None yet
Milestone
1.2
Development

Successfully merging this pull request may close these issues.

Boolean indexing and assignment from python list broken
5 participants
@paxcodes @phofl @MarcoGorelli @ivanovmg @arw2019