MAINT: Updated tick and category test formatting by madphysicist · Pull Request #7993 · matplotlib/matplotlib

madphysicist · 2017-01-30T17:11:07Z

Rearranged tests in lib/matplotlib/tests/test_ticker.py and made a couple of very minor changes to lib/matplotlib/tests/test_category.py. Based on comment by @story645 in PRs #7482 and #7965.

This PR should probably be merged before either of the others so I can rebase them onto it and clean things up.

story645 · 2017-01-30T18:45:44Z

lib/matplotlib/tests/test_ticker.py

+        assert formatter.format_pct(x, display_range) == expected
+
+
+class TestEndFormatter(object):


typo: EngFormatter

story645 · 2017-01-30T18:52:39Z

lib/matplotlib/tests/test_category.py

+        np.array(['1', '11', '3']),
+        [b'1', b'11', b'3'],
+        np.array([b'1', b'11', b'3'])],
+    ]


apparently an extra ] here messing up travis?

story645 · 2017-01-30T18:53:28Z

😄 thanks!

madphysicist · 2017-01-30T19:13:44Z

Thanks for looking through the PRs. I fixed up the issues you found.

madphysicist · 2017-01-30T20:00:32Z

I am pretty sure that this PR is ready to go. The failing tests appear to be caused by an external source.

madphysicist · 2017-01-31T06:49:54Z

No more failing tests.

madphysicist · 2017-01-31T16:08:32Z

@dstansby. I've been seeing [MRG+1] a lot recently on PR titles. What does it mean?

story645 · 2017-01-31T16:22:45Z

It means your PR has been approved by one reviewer (matplotlib asks for 2 approvals before commiting). http://matplotlib.org/devel/coding_guide.html#pr-review-guidelines

madphysicist · 2017-01-31T17:23:23Z

Can you be the other one?

story645 · 2017-01-31T18:00:45Z

lib/matplotlib/tests/test_ticker.py

+        cases, especially the case when no SI prefix is present, for values in
+        [1, 1000).
+
+        Should not raise exceptions.


don't need this comment.

story645 · 2017-01-31T18:01:47Z

lib/matplotlib/tests/test_ticker.py

+
+        Should not raise exceptions.
+        """
+        unitless = mticker.EngFormatter()


these should probably be two separate tests (unitless and with unit) and possibly parameterized

story645 · 2017-01-31T18:02:35Z

Sorry thought I approved. Have two comments, but if you wanna punt those to a different improve engineering test PR, let me know and I'll merge.

madphysicist · 2017-01-31T19:03:51Z

@story645 I made the improvements here. I kept is as a single parametrized test, removed the extra comment and also took care of some extra whitespace in ticker.py.

madphysicist · 2017-02-01T02:42:58Z

Not an actual failure, Mac test just didn't run.

QuLogic · 2017-02-01T03:34:02Z

With #7973 merged, this has conflicts. When you rebase, please be sure to leave out any empty cleanup decorators.

story645 · 2017-02-01T03:34:43Z

I think on the rebase or something, you accidentally introduced a conflict. Please resolve.

madphysicist · 2017-02-01T03:44:03Z

@QuLogic Should I replace @cleanup(style='classic') with @pytest.mark.style('classic'), and make all similar changes that make sense?

QuLogic · 2017-02-01T03:48:28Z

Classic is the default now, it's not necessary if that's really what's being used (and I realize I forgot to remove a couple.)

madphysicist · 2017-02-01T03:58:32Z

In that case I will remove those too.

madphysicist · 2017-02-01T04:00:29Z

Just to make triple sure, I should make the replacement for 'default' though?

QuLogic · 2017-02-01T04:01:56Z

That's correct.

madphysicist · 2017-02-01T04:17:56Z

Done. I ended up parametrizing a couple of the tests and adding one pytest.mark.style('default') that I don't think was there before to make things pass. Should be good now.

madphysicist · 2017-02-01T16:25:23Z

Again looks like Mac test didn't run. No actual failures.

madphysicist · 2017-02-02T05:11:48Z

I've closed and reopened the PR to restart Travis and hopefully get the Mac test to run, but if you approve, this PR is ready to go.

QuLogic · 2017-02-02T05:22:45Z

lib/matplotlib/tests/test_ticker.py

+                               9.441, 12.588])
+        assert_almost_equal(loc.tick_values(-7, 10), test_value)
+
+    def test_MultipleLocator_set_params(self):


Seems like you forgot to remove the class in this one.

QuLogic · 2017-02-02T05:53:27Z

lib/matplotlib/tests/test_ticker.py

+        fmt = ax.xaxis.get_major_formatter()
+        fmt.set_locs(ax.xaxis.get_majorticklocs())
+        show_major_labels = [fmt(x) != '' for x in
+                                            ax.xaxis.get_majorticklocs()]


Not aligned properly; better to wrap at for.

QuLogic · 2017-02-02T05:55:39Z

lib/matplotlib/tests/test_ticker.py

+    @pytest.mark.parametrize(
+            'labelOnlyBase, exponent, locs, positions, expected', param_data)
+    @pytest.mark.parametrize('base', base_data)
+    def test_LogFormatterExponent(self, labelOnlyBase, base, exponent,


Class name is still here.

QuLogic · 2017-02-02T05:57:10Z

lib/matplotlib/tests/test_ticker.py

+
+
+class TestLogFormatterSciNotation(object):
+    testdata = [


Why testdata and not test_data like the others?

I wasn't sure if it would cause any problems if I started it with test_.

We'll see how the tests go, then, but I'd guess this shouldn't be a problem since it isn't callable.

madphysicist · 2017-02-02T06:16:39Z

@QuLogic Made the following changes:

Removed 2 class names in method names.
Renamed testdata to test_data.
Wrapped list comprehension with for on new line.

madphysicist · 2017-02-02T20:53:26Z

Ready to go?

This was referenced Jan 30, 2017

ENH: Added a new Formatter to mpl.ticker and some example recipes #7482

Closed

ENH: Fixed PercentFormatter usage with latex #7965

Merged

story645 reviewed Jan 30, 2017

View reviewed changes

madphysicist force-pushed the test-fixture-fixup branch 3 times, most recently from 6d2c93f to 37592a7 Compare January 30, 2017 22:16

madphysicist force-pushed the test-fixture-fixup branch 2 times, most recently from f4b6f48 to 37592a7 Compare January 31, 2017 14:50

dstansby approved these changes Jan 31, 2017

View reviewed changes

dstansby changed the title ~~MAINT: Updated tick and category test formatting~~ [MRG+1] MAINT: Updated tick and category test formatting Jan 31, 2017

story645 approved these changes Jan 31, 2017

View reviewed changes

story645 reviewed Jan 31, 2017

View reviewed changes

madphysicist force-pushed the test-fixture-fixup branch 2 times, most recently from 9d8d3ab to e3f721f Compare January 31, 2017 19:03

madphysicist force-pushed the test-fixture-fixup branch 2 times, most recently from 85ee55b to 5211dc9 Compare January 31, 2017 19:49

madphysicist force-pushed the test-fixture-fixup branch from 5211dc9 to f9f2cb1 Compare February 1, 2017 04:15

madphysicist closed this Feb 2, 2017

madphysicist reopened this Feb 2, 2017

QuLogic requested changes Feb 2, 2017

View reviewed changes

MAINT: Updated tick and category test formatting

3109ce9

madphysicist force-pushed the test-fixture-fixup branch from f9f2cb1 to 3109ce9 Compare February 2, 2017 06:15

QuLogic approved these changes Feb 2, 2017

View reviewed changes

QuLogic added this to the 2.1 (next point release) milestone Feb 2, 2017

QuLogic changed the title ~~[MRG+1] MAINT: Updated tick and category test formatting~~ MAINT: Updated tick and category test formatting Feb 2, 2017

QuLogic merged commit 7ffcca9 into matplotlib:master Feb 2, 2017

madphysicist deleted the test-fixture-fixup branch February 2, 2017 22:29

		assert formatter.format_pct(x, display_range) == expected


		class TestEndFormatter(object):

Uh oh!

Conversation

madphysicist commented Jan 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

story645 Jan 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

story645 commented Jan 30, 2017

Uh oh!

madphysicist commented Jan 30, 2017

Uh oh!

madphysicist commented Jan 30, 2017

Uh oh!

madphysicist commented Jan 31, 2017

Uh oh!

madphysicist commented Jan 31, 2017

Uh oh!

story645 commented Jan 31, 2017

Uh oh!

madphysicist commented Jan 31, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

story645 commented Jan 31, 2017

Uh oh!

madphysicist commented Jan 31, 2017

Uh oh!

madphysicist commented Feb 1, 2017

Uh oh!

QuLogic commented Feb 1, 2017

Uh oh!

story645 commented Feb 1, 2017

Uh oh!

madphysicist commented Feb 1, 2017

Uh oh!

QuLogic commented Feb 1, 2017

Uh oh!

madphysicist commented Feb 1, 2017

Uh oh!

madphysicist commented Feb 1, 2017

Uh oh!

QuLogic commented Feb 1, 2017

Uh oh!

madphysicist commented Feb 1, 2017

Uh oh!

madphysicist commented Feb 1, 2017

Uh oh!

madphysicist commented Feb 2, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

madphysicist commented Feb 2, 2017

Uh oh!

madphysicist commented Feb 2, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

madphysicist commented Jan 30, 2017 •

edited

Loading

story645 Jan 30, 2017 •

edited

Loading