-
Notifications
You must be signed in to change notification settings - Fork 358
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
BUG&TST: Fix area plot for Series and add test #704
BUG&TST: Fix area plot for Series and add test #704
Conversation
Softagram Impact Report for pull/704 (head commit: aa9fed6)⭐ Change Overview
📄 Full report
Give feedback on this report to [email protected] |
Codecov Report
@@ Coverage Diff @@
## master #704 +/- ##
==========================================
+ Coverage 93.86% 93.86% +<.01%
==========================================
Files 32 32
Lines 5544 5545 +1
==========================================
+ Hits 5204 5205 +1
Misses 340 340
Continue to review full report at Codecov.
|
Thanks!!!! |
@@ -90,8 +90,9 @@ def get_sampled(self, data): | |||
sampled = data._sdf.sample(fraction=float(self.fraction)) | |||
return DataFrame(data._internal.copy(sdf=sampled)).to_pandas() | |||
elif isinstance(data, Series): | |||
scol = data._kdf._internal.data_scols[0] |
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.
oops, it should be data._scol
so that it respects the expression stored in Series. Let me fix it quick.
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.
oops... i didn't see this option in Series.py
... my bad, sorry for this, i see it is not fixed yet, i will just create a small PR to fix it quick and validate if plot still works. @HyukjinKwon
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.
ah, I just made a one line fix just now - I found this comment after opening the PR :D. It's fine!
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.
😅 oh, sorry, i waked up and saw the comment, and found it hadn't been fixed yet in master branch, i thought you might forget this issue, so i opened a small PR for this ... but thanks for the tip, appreciate it!
There seems a little bug in SamplePlot that failed area plot. It is due to after sampling, since
scol
is not assigned, it will return a full DataFrame instead of Series. And looks like area plot turns good as well after fixing it.Before the bug fixing, the area plot looks like: