-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
Split test multi #20669
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
Split test multi #20669
Conversation
Hello @xchoudhury! Thanks for updating the PR. Cheers ! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated on April 13, 2018 at 05:02 Hours UTC |
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.
pls rebase. want to split slightly less, made some comments. also want to remove any unused code in each module. make sure the linter passes on this.
|
||
class TestConstructor(Base): | ||
_holder = MultiIndex | ||
_compat_props = ['shape', 'ndim', 'size', 'itemsize'] |
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.
so things that are not used in a particular module need not be repeated here (e.g. in the constructor) and/or properties in the class.
We want to actually remove the need for the class entirely. You could do that here as well (or in a next pass PR).
This is a little trickier because need to turn things (like self.index) into fixtures. so ok with doing this later.
def create_index(self): | ||
return self.index | ||
|
||
def test_set_name_methods(self): |
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.
rather these go with the names methods
self.index.labels = new_labels | ||
|
||
def test_set_levels(self): | ||
# side note - you probably wouldn't want to use levels and labels |
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.
similar this should go with get_levels
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.
see below get/set names/levels/labels can all be in 1 file
check_dtype=True) | ||
|
||
def test_set_labels(self): | ||
# side note - you probably wouldn't want to use levels and labels |
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.
get/set lables/levels can be in 1 file
result = sorted_idx.slice_locs('bar', 'baz') | ||
assert result == (2, 4) | ||
|
||
def test_slice_locs_not_contained(self): |
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.
moving slice, get_loc, get_indexer into a test_indexing
|
||
import numpy as np | ||
|
||
from pandas import (Index, MultiIndex) |
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 be just test_sorting
|
||
def create_index(self): | ||
return self.index | ||
|
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.
move all of the constructors (from_product, from_array, from_tuples) into a test_constructor
self.setup_indices() | ||
|
||
def create_index(self): | ||
return self.index |
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.
move this with test_format
minor_labels = np.array([0, 1, 0, 1, 0, 1]) | ||
self.index_names = ['first', 'second'] | ||
self.indices = dict(index=MultiIndex(levels=[major_axis, minor_axis], | ||
labels=[major_labels, minor_labels |
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.
you can put unique, isin, duplicates in a single file
make sure you rebase on origin/master |
@xchoudhury I see you've done a lot of good work on splitting out the multi index tests. Would you like any help polishing this off? I'd be happy to assist. |
Checklist for the pandas documentation sprint (ignore this if you are doing
an unrelated PR):
scripts/validate_docstrings.py <your-function-or-method>
git diff upstream/master -u -- "*.py" | flake8 --diff
python doc/make.py --single <your-function-or-method>
Please include the output of the validation script below between the "```" ticks:
If the validation script still gives errors, but you think there is a good reason
to deviate in this case (and there are certainly such cases), please state this
explicitly.
Checklist for other PRs (remove this part if you are doing a PR for the pandas documentation sprint):
git diff upstream/master -u -- "*.py" | flake8 --diff