-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
TST: Move plotting related tests to tests/plotting #13621
Conversation
@sinhrks Looks good! While we are in splitting modus, maybe also split the tests for 'other' dataframe/series methods (hist/boxplot) on the one hand, and the 'misc' functions on the other hand? |
|
||
|
||
""" | ||
These tests are for ``DataFrame.hist``, ``DataFrame.boxplot`` and | ||
other miscellaneous plots. | ||
`Dataframe.plot`` and ``Series.plot`` are tested in test_graphics.py | ||
`Dataframe.plot`` and ``Series.plot`` are tested in | ||
test_frame.py and test_series.py |
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.
In this file it makes maybe more sense to split the tests in classes regarding hist vs boxplot tests, instead of series vs dataframe tests?
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.
good idea. How about the following structure?
.plot
(previouslytest_graphics
)test_series
test_frame
test_groupby
- others (previously
test_graphics_others
)test_legacy_hist
test_legacy_boxplot
test_misc
CC: @TomAugspurger
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.
Only issue with that is the tests that use the data structures from setUp
. Could make a common base class for those.
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.
@TomAugspurger I don't think that is a problem, indeed easy to make a base class, and there is already a common file
@sinhrks Is there a reason to call it test_legacy_hist
instead of test_hist
? As we did not really officially deprecate them? And they are still used in the docs (but that is a very minor comment :-))
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.
Not specific, I intended to clarify the differences between normal plot
and boxplot/hist
methods. Using boxplot_method
and others.
And travis is failing because of lint issues |
Current coverage is 84.38%@@ master #13621 diff @@
==========================================
Files 142 142
Lines 51235 51235
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
Hits 43235 43235
Misses 8000 8000
Partials 0 0
|
Updated to use following file names:
|
I'm willing to rebase after #13147:) |
Question: do we actually want it in |
Whichever possible, |
I actually meant |
Yeah it'll be a goal if there is no objections. Shall we move |
Would like to decide the (temporary) namespace to work on. I'm +1 for |
That's fine for me. This is ready to merge then? |
@jorisvandenbossche yes, related files are not updated after the previous rebase. |
Merging then! |
git diff upstream/master | flake8 --diff
Because
test_graphics
is getting huge, split them based on data types undertests/plotting
to make visualization related tests easier.