Skip to content

Conversation

@charlesdong1991
Copy link
Contributor

@charlesdong1991 charlesdong1991 commented Aug 24, 2019

I came across test_series_plot.py and saw the existing plots might lack sanity check.
e.g. if a test s.plot.line() is added, I think it's also good to have sanity check to see if s.plot(kind='line') outputs the correct fig (in principle, it of course should).

The plots I added are: line, bar, hist and pie.

@charlesdong1991 charlesdong1991 changed the title Add sanity checks for some series plot TST: Add sanity checks for some series plot Aug 24, 2019
@codecov-io
Copy link

codecov-io commented Aug 24, 2019

Codecov Report

Merging #687 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #687      +/-   ##
==========================================
+ Coverage   93.52%   93.54%   +0.01%     
==========================================
  Files          32       32              
  Lines        5455     5455              
==========================================
+ Hits         5102     5103       +1     
+ Misses        353      352       -1
Impacted Files Coverage Δ
databricks/koalas/plot.py 94.61% <0%> (+0.33%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ba6c83...bf91623. Read the comment docs.

@softagram-bot
Copy link

Softagram Impact Report for pull/687 (head commit: bf91623)

⭐ Change Overview

Showing the changed files, dependency changes and the impact - click for full size
(Open in Softagram Desktop for full details)

📄 Full report

Give feedback on this report to [email protected]

@HyukjinKwon HyukjinKwon merged commit d1b8bfb into databricks:master Aug 25, 2019
@HyukjinKwon
Copy link
Member

What does TST stand for BTW :D?

@HyukjinKwon
Copy link
Member

@itholic, BTW you can start to work on such missing test coverage too.

@itholic
Copy link
Contributor

itholic commented Aug 25, 2019

@HyukjinKwon Thanks Hyukjin, I'll try and share it when the results come out! :)

@charlesdong1991
Copy link
Contributor Author

What does TST stand for BTW :D?

it was short for test xD, just wanna intuitively show this PR is just test fix.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants