[Fix issue][#3511]lineplot of empty dataframe with hue in seaborn 0.13.0 #3569
Conversation
Corresponds to the case where grouping_var has None object.
Update _base.py
|
Thanks for taking a crack at this. Can you please add a test case that would fail with the current behavior (and pass with your fix)? Thanks! |
|
Thank you for your comment! |
Add a test case to test_relational.py and a fixture empty dataframe.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #3569 +/- ##
=======================================
Coverage 98.34% 98.34%
=======================================
Files 75 75
Lines 24626 24629 +3
=======================================
+ Hits 24218 24222 +4
+ Misses 408 407 -1
|
tests/conftest.py
Outdated
| @pytest.fixture | ||
| def empty_df(long_df): | ||
| return long_df.drop(long_df.index) |
There was a problem hiding this comment.
I don't think we need another fixture for this, you can just empty out long_df where you pass the data to lineplot in the relevant test (not that it matters too much, but long_df.iloc[:0] is probably more efficient than dropping every index value).
seaborn/_base.py
Outdated
| key = levels.get(var) if levels.get(var) is not None else [] | ||
| grouping_keys.append(key) |
There was a problem hiding this comment.
Total nitpick, but the duplicate call to levels.get above looks cumbersome, maybe move the ternary down here? e.g.
| key = levels.get(var) if levels.get(var) is not None else [] | |
| grouping_keys.append(key) | |
| key = levels.get(var) | |
| grouping_keys.append([] if key is None else key) |
- Remove unnecessary fixture. - Refactoring.
|
Thank you for your review! |
|
Thanks @Noko-Github I think it's possible that this could also be fixed upstream by making the default |
|
Btw if you make the PR message say "Closes #issue" or "Fixes #issue" it will close the issue automatically when merged. |
Corresponds to the case where grouping_var has None object.
Related issue: #3511