CLN: Continued indexes/test_base.py cleanup #20813

WillAyd · 2018-04-24T18:21:32Z

progress towards #20812

WillAyd · 2018-04-24T18:22:18Z

pandas/tests/indexes/test_base.py

-        # preserve names
+    @pytest.mark.parametrize("first_nm,sec_nm,exp_nm", [
+        ('A', 'A', 'A'), ('A', 'B', None), (None, 'B', None)])
+    def test_intersection_name_preservation2(self, first_nm, sec_nm, exp_nm):


Kept as a separate test for now, but ideally should be combined with the test above as they cover the same topics. Waiting until I get around to replacing the index class attrs with fixtures to explore in more detail

WillAyd · 2018-04-24T18:24:00Z

pandas/tests/indexes/test_base.py

-    def test_map_tseries_indices_return_index(self):
-        date_index = tm.makeDateIndex(10)
+    @pytest.mark.parametrize("attr", [
+        'makeDateIndex', 'makePeriodIndex', 'makeTimedeltaIndex'])


Will most likely replace with a fixture in the future, but made sense to parametrize for now

WillAyd · 2018-04-24T18:25:01Z

pandas/tests/indexes/test_base.py

-    def test_difference(self):
-
+    @pytest.mark.parametrize("second_nm,exp", [
+        (None, None), ('name', 'name')])


Might be over-engineering the parameters in this case, but figure if we wanted to expand to cover more scenarios this is a better framework to do so

WillAyd · 2018-04-24T18:26:05Z

pandas/tests/indexes/test_base.py

+    @pytest.mark.parametrize("attr,exp", [
+        ('strIndex', False), ('boolIndex', False), ('catIndex', False),
+        ('intIndex', True), ('dateIndex', False), ('floatIndex', True)])
+    def test_is_numeric(self, attr, exp):


This and the two tests below it did not consistently test all of the index types so I've added where missing for better coverage. Should ultimately be replaced with fixtures but same concept will apply

WillAyd · 2018-04-24T18:26:36Z

pandas/tests/indexes/test_base.py

@@ -1142,19 +1106,21 @@ def test_format(self):
            expected = [str(index[0])]
            assert formatted == expected

+        self.strIndex[:0].format()
+
+    def test_format_missing(self, nulls_fixture):


Was only testing np.nan and None before but figured this was a good use case for the new nulls fixture

WillAyd · 2018-04-24T18:28:31Z

pandas/tests/indexes/test_base.py

-        formatted = index.format()
-        expected = [str(index[0]), str(index[1]), u('NaN')]
-        assert formatted == expected
+    def test_format_missing_obj(self, nulls_fixture):


Trying to balance too many changes but am leaning towards combining this with the above test. Both are pretty similar but leave strange gaps in what they test across the float and object types

jreback

looks good. if can fix the naming then looks good to merge

jreback · 2018-04-24T19:05:03Z

pandas/tests/indexes/test_base.py


-        # preserve names
+    @pytest.mark.parametrize("first_nm,sec_nm,exp_nm", [


ideally can you spell things out a bit more, e.g. first_name, second_name, expected_name are just fine

Would you prefer generally using result and expected over res and exp? I've seen both and typically use the latter but if we have a preference we could start being explicit about those standards (maybe in TESTING_README)

I really prefer result, expected

the code base mostly has these, but yes res, exp are also used. we should standardize and maybe check this in lint. (you can create an issue if you'd like)

jreback · 2018-04-24T19:05:29Z

pandas/tests/indexes/test_base.py

-        second.name = 'B'
-        intersect = first.intersection(second)
-        assert intersect.name is None
+    @pytest.mark.parametrize("idx2,keeps_nm", [


so keeps_nm -> keeps_name and normalize the input params, e.g. index (or index2 or other) as appropriate

jreback · 2018-04-24T19:07:16Z

pandas/tests/indexes/test_base.py

        union = first.union(second)
        assert tm.equalContents(union, everything)

+    @pytest.mark.parametrize("klass", [


this notion of klass we have all over the place, sometimes its also called box (not necessary to do here, just letting you know, prob best to update the issue you created)

jreback · 2018-04-24T19:08:35Z

pandas/tests/indexes/test_base.py

-
-        first = Index([], name='A')
-        second = Index(list('ab'))
+    @pytest.mark.parametrize("first,second,nm", [


maybe could replace this with 2 parameterize (for a cartesian product of the inputs)

jreback · 2018-04-24T19:09:08Z

pandas/tests/indexes/test_base.py

@@ -967,18 +933,14 @@ def test_append_multiple(self):
        result = index.append([])
        tm.assert_index_equal(result, index)

-    def test_append_empty_preserve_name(self):
+    @pytest.mark.parametrize("nm,exp", [


name, expected

jreback · 2018-04-24T19:09:27Z

pandas/tests/indexes/test_base.py

-    def test_difference(self):
-
+    @pytest.mark.parametrize("second_nm,exp", [
+        (None, None), ('name', 'name')])


jreback · 2018-04-24T19:10:07Z

pandas/tests/indexes/test_base.py

-        formatted = index.format()
-        expected = [str(index[0]), str(index[1]), u('NaN')]
-        assert formatted == expected
+    def test_format_missing_obj(self, nulls_fixture):


jreback · 2018-04-24T19:12:06Z

pandas/tests/indexes/test_base.py

+        (Index([3, 4, 5, 6, 7], name="idx"), True),  # preserve same name
+        (Index([3, 4, 5, 6, 7], name="other"), False),  # drop diff names
+        (Index([3, 4, 5, 6, 7]), False)])
+    def test_intersection_name_preservation(self, idx2, keeps_nm):


side note, we prob need to split out test_base into multiple files its growing pretty big (do this as a separate step), these might be things like: test_indexing.py, test_missing.py, test_set_ops.py etc, in essence mirroring the various sub-files we do for series ops

Yep - I have this as the last TODO in the referenced issue so we don't lose sight of it

codecov · 2018-04-25T00:33:18Z

Codecov Report

Merging #20813 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #20813   +/-   ##
=======================================
  Coverage   91.77%   91.77%           
=======================================
  Files         153      153           
  Lines       49265    49265           
=======================================
  Hits        45215    45215           
  Misses       4050     4050

Flag	Coverage Δ
#multiple	`90.16% <ø> (ø)`	⬆️
#single	`41.86% <ø> (ø)`	⬆️

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 8bee97a...1acfda1. Read the comment docs.

jreback · 2018-04-27T10:56:09Z

thanks @WillAyd

CLN: Continued indexes/test_base.py cleanup

1acfda1

WillAyd commented Apr 24, 2018

View reviewed changes

jreback added Testing pandas testing functions or related to the test suite Indexing Related to indexing on series/frames, not to indexes themselves labels Apr 24, 2018

jreback requested changes Apr 24, 2018

View reviewed changes

WillAyd added 8 commits April 24, 2018 19:35

Replaced res/exp with result/expected

be271ab

Replaced idx with index

c5aebe1

Replaced nm with name

2f90fb0

Combined format_missing tests

1d09b5d

Updated parameters for test_union_name_preservation

45d305d

More exp -> expected updates

061b227

LINT fixup

24f618f

Merge remote-tracking branch 'upstream/master' into idx-clnup2

25af5dc

WillAyd mentioned this pull request Apr 25, 2018

Established Standards for Tests #20820

Closed

jreback added this to the 0.23.0 milestone Apr 27, 2018

jreback approved these changes Apr 27, 2018

View reviewed changes

jreback merged commit 96f2f57 into pandas-dev:master Apr 27, 2018

WillAyd mentioned this pull request Apr 30, 2018

TST: Cleanup tests/indexes/test_base.py #20812

Closed

8 tasks

WillAyd deleted the idx-clnup2 branch May 14, 2018 21:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLN: Continued indexes/test_base.py cleanup #20813

CLN: Continued indexes/test_base.py cleanup #20813

WillAyd commented Apr 24, 2018

WillAyd Apr 24, 2018

WillAyd Apr 24, 2018

WillAyd Apr 24, 2018

jreback Apr 24, 2018

WillAyd Apr 24, 2018

WillAyd Apr 24, 2018

WillAyd Apr 24, 2018

jreback Apr 24, 2018

jreback left a comment

jreback Apr 24, 2018

WillAyd Apr 24, 2018

jreback Apr 24, 2018

jreback Apr 24, 2018

jreback Apr 24, 2018

jreback Apr 24, 2018

jreback Apr 24, 2018

jreback Apr 24, 2018

jreback Apr 24, 2018

jreback Apr 24, 2018

WillAyd Apr 24, 2018

codecov bot commented Apr 25, 2018 •

edited

Loading

jreback commented Apr 27, 2018


		# preserve names
		@pytest.mark.parametrize("first_nm,sec_nm,exp_nm", [

CLN: Continued indexes/test_base.py cleanup #20813

CLN: Continued indexes/test_base.py cleanup #20813

Conversation

WillAyd commented Apr 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 25, 2018 • edited Loading

Codecov Report

jreback commented Apr 27, 2018

codecov bot commented Apr 25, 2018 •

edited

Loading