-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
TST/CLN: empty DataFrames and some 'empty' Series #25690
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #25690 +/- ##
==========================================
+ Coverage 91.28% 91.28% +<.01%
==========================================
Files 173 173
Lines 52965 52965
==========================================
+ Hits 48348 48349 +1
+ Misses 4617 4616 -1
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #25690 +/- ##
==========================================
+ Coverage 91.28% 91.28% +<.01%
==========================================
Files 173 173
Lines 52965 52965
==========================================
+ Hits 48348 48349 +1
+ Misses 4617 4616 -1
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #25690 +/- ##
==========================================
+ Coverage 91.47% 91.48% +<.01%
==========================================
Files 175 175
Lines 52885 52885
==========================================
+ Hits 48379 48380 +1
+ Misses 4506 4505 -1
Continue to review full report at Codecov.
|
this is fine. is it possible to have a code_checks rule to detect these (slightly tricky as you only want to disallow these in an assignment (and not a lambda). |
lambda: DataFrame(data=[]), | ||
lambda: DataFrame(data=(x for x in [])), | ||
# these are NOT empty DataFrames | ||
pytest.param(lambda: DataFrame([[]]), marks=pytest.mark.xfail( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to parametrize these in a separate test rather than supplying mark here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was just cautious of making actual behaviour tested and then tested behaviour becoming the accepted behaviour because it's tested. I'm not sure if the index differences are intentional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah i agree, can you separate these cases out
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you update this and merge master, ping on green.
can you update |
have not looked into this yet, will circle back round after revisiting my other open PRs. |
thanks @simonjayhawkins yeah if you can think of a nice way to add to code_checks would be great. |
xref #24886 (comment)
have included Series constructed with an empty dict.
Series constructed with an empty list, tuple or generator are not the same as Series() or Series({}), so have not included them in this pass. (although for many tests the difference in the Index is not relevant)