ENH: Fixed PercentFormatter usage with latex #7965

madphysicist · 2017-01-27T22:06:24Z

This PR addresses a nagging issue that I have had with PercentFormatter, namely that it does not handle LaTeX labels correctly unless preconfigured to do so. The proposed solution allows the labels to be generated correctly with the same formatter even if rcParams['text.usetex'] changes between calls.

I also added it to the big formatter example and modified one of the existing pylab examples where it clearly belongs.

This PR also includes the of IDs to test_percentformatter that I made in #7482.

madphysicist · 2017-01-27T22:07:38Z

@story645: given your concern with PercentFormatter, I think you might be a good person to have reviewing this.

NelleV · 2017-01-27T23:12:49Z

lib/matplotlib/ticker.py


    def convert_to_pct(self, x):
        return 100.0 * (x / self.xmax)

+    def get_symbol(self):


Can you make this function private? If possible, we try to avoid adding more elements to the public API that aren't absolutely necessary.

How about making self.symbol into a property since it's already public and removing the need for this method entirely?

sounds good.

QuLogic · 2017-01-27T23:23:50Z

examples/ticks_and_spines/tick-formatters.py

@@ -22,7 +22,7 @@ def setup(ax):


 plt.figure(figsize=(8, 5))


Figure should be made a little bigger to fit the new Axes.

QuLogic · 2017-01-27T23:24:49Z

lib/matplotlib/tests/test_ticker.py

@@ -536,12 +536,66 @@ def test_formatstrformatter():


 @pytest.mark.parametrize('xmax, decimals, symbol, x, display_range, expected',
-                         percentformatter_test_cases)


Why move this down but not delete percentformatter_test_cases?

Accidentally copied instead of cutting, then forgot because the tests passed. Will fix.

same comment as on other PR, maybe keep the testcase and id outside of the decorator and just refer to 'em?

story645 · 2017-01-29T02:21:16Z

examples/pylab_examples/histogram_percent_demo.py

 # Set the formatter
-plt.gca().yaxis.set_major_formatter(formatter)
+plt.gca().yaxis.set_major_formatter(PercentFormatter(xmax=1))


I know this is beyond the scope of this doc, but is there anyway to do this same thing without a plt.gca().yaxis? Also comment should possibly explain what xmax means here

This is not out of scope at all if you are OK with an extra call to plt.subplots().

story645 · 2017-01-29T02:23:02Z

lib/matplotlib/tests/test_ticker.py

@@ -536,12 +536,66 @@ def test_formatstrformatter():


 @pytest.mark.parametrize('xmax, decimals, symbol, x, display_range, expected',
-                         percentformatter_test_cases)


same comment as on other PR, maybe keep the testcase and id outside of the decorator and just refer to 'em?

madphysicist · 2017-01-30T16:02:00Z

I have responded to all the review comments. The lists of test parameters was made into separate references in #7993.

madphysicist · 2017-01-30T19:09:09Z

I am pretty sure that the Appveyor failure is not something I did, but I could be very wrong about that.

story645 · 2017-01-30T19:14:41Z

You could click through and see if the error has anything to do with you, but travis and appveyor have also been messed around with a lot lately.

madphysicist · 2017-01-30T19:48:45Z

Well, #7528 started getting the same error after I rebased, so I am pretty sure it's something on master. Blame says @QuLogic is the last person to modify that particular test about 10 days ago, but it may not be the test itself that is causing the failure.

QuLogic · 2017-02-01T00:00:36Z

examples/ticks_and_spines/tick-formatters.py

@@ -21,8 +21,8 @@ def setup(ax):
    ax.patch.set_alpha(0.0)


-plt.figure(figsize=(8, 5))
-n = 6
+plt.figure(figsize=(9, 5))


You've changed the width, not the height.

QuLogic · 2017-02-01T00:00:39Z

lib/matplotlib/tests/test_ticker.py

@@ -511,35 +511,66 @@ def test_formatstrformatter():
    assert '002-01' == tmp_form(2, 1)


-percentformatter_test_cases = (


I thought @story645 suggested keeping this.

Got sorted out in #7993

They'll conflict then; which should be merged first?

#7993 please. I'd like to rebase all my open PRs onto it. It's ready to go, just waiting for Mac test to run.

madphysicist · 2017-02-01T03:00:24Z

@QuLogic. I fixed the size issue. In addition to #7993, the order of this PR with respect to #7482 matters as well since they both add examples to the big formatter demo. I updated the same resizing issue in #7482 as well. #7482 should come after this PR. Just out of curiosity, is there some mechanism in GitHub for specifying the order in which PRs should be merged?

madphysicist · 2017-02-03T06:37:07Z

Fixed PEP8 errors, should be ready to merge when tests are done. This PR needs to come before #7482 because of the conflicts between them.

madphysicist · 2017-02-03T16:48:55Z

I suspect that the break may not be related to my tests...

madphysicist · 2017-02-04T05:00:36Z

ready to merge with one more approval.

QuLogic · 2017-02-05T02:31:27Z

examples/pylab_examples/histogram_percent_demo.py

-# default labels by 100, making them all percentages
-formatter = FuncFormatter(to_percent)
+# Make a normed histogram. It'll be multiplied by 100 later.
+plt.hist(x, bins=50, normed=True, axes=ax)


Doesn't ax.hist work here?

it's a pylab example so I think it has to be plt.hist

@madphysicist already changed it to fig, ax, so I don't see why.

Technically, plt.* is already not pylab, so I will go ahead and make the change. I am pretty sure that there are plenty of pylab examples that demo the OO API.

I agree @madphysicist a good deal of time was spent removing the use of pylab from all the examples last year. In my opinion most of the pylab examples should be moved to other folders with more meaningful names as they are cleaned up and improved (But I am out of touch on what the current plan is for restructuring the examples)

QuLogic · 2017-02-05T02:42:19Z

lib/matplotlib/tests/test_ticker.py

    @pytest.mark.parametrize(
            'xmax, decimals, symbol, x, display_range, expected',
            percent_data, ids=percent_ids)
-    def test_percentformatter(self, xmax, decimals, symbol,
+    def test_basic(self, xmax, decimals, symbol,
                              x, display_range, expected):


Not aligned any more.

Added an example to the big formatter example Added test for latex correctness Modified existing pylab example

NelleV reviewed Jan 27, 2017

View reviewed changes

QuLogic reviewed Jan 28, 2017

View reviewed changes

story645 reviewed Jan 29, 2017

View reviewed changes

madphysicist mentioned this pull request Jan 30, 2017

MAINT: Updated tick and category test formatting #7993

Merged

story645 approved these changes Jan 30, 2017

View reviewed changes

madphysicist force-pushed the pct-formatter-updates branch 2 times, most recently from 217536d to c33266b Compare January 30, 2017 20:35

madphysicist mentioned this pull request Jan 31, 2017

BUG: % crashes saving figure with tex enabled #6886

Closed

story645 changed the title ~~ENH: Fixed PercentFormatter usage with latex~~ [MRG+1] ENH: Fixed PercentFormatter usage with latex Jan 31, 2017

QuLogic reviewed Feb 1, 2017

View reviewed changes

madphysicist force-pushed the pct-formatter-updates branch from c33266b to fde20d2 Compare February 1, 2017 02:56

madphysicist closed this Feb 2, 2017

madphysicist reopened this Feb 2, 2017

madphysicist force-pushed the pct-formatter-updates branch 2 times, most recently from 9f1aaed to 98211e4 Compare February 3, 2017 05:27

madphysicist mentioned this pull request Feb 3, 2017

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

Closed

madphysicist force-pushed the pct-formatter-updates branch from 98211e4 to 43a6bd6 Compare February 3, 2017 06:35

QuLogic reviewed Feb 5, 2017

View reviewed changes

ENH: Fixed PercentFormatter usage with latex

6add6ce

Added an example to the big formatter example Added test for latex correctness Modified existing pylab example

madphysicist force-pushed the pct-formatter-updates branch from 43a6bd6 to 6add6ce Compare February 6, 2017 16:53

QuLogic changed the title ~~[MRG+1] ENH: Fixed PercentFormatter usage with latex~~ ENH: Fixed PercentFormatter usage with latex Feb 6, 2017

QuLogic merged commit 91261e7 into matplotlib:master Feb 6, 2017

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

madphysicist deleted the pct-formatter-updates branch February 7, 2017 03:52

voidstarstar mentioned this pull request Apr 24, 2024

[ENH]: Format PercentFormatter based on usetex value #28126

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: Fixed PercentFormatter usage with latex #7965

ENH: Fixed PercentFormatter usage with latex #7965

madphysicist commented Jan 27, 2017 •

edited

Loading

madphysicist commented Jan 27, 2017

NelleV Jan 27, 2017

madphysicist Jan 28, 2017

NelleV Jan 28, 2017

QuLogic Jan 27, 2017

QuLogic Jan 27, 2017

madphysicist Jan 28, 2017

story645 Jan 29, 2017

story645 Jan 29, 2017

madphysicist Jan 30, 2017

story645 Jan 29, 2017

madphysicist commented Jan 30, 2017 •

edited

Loading

madphysicist commented Jan 30, 2017

story645 commented Jan 30, 2017

madphysicist commented Jan 30, 2017

QuLogic Feb 1, 2017

QuLogic Feb 1, 2017

story645 Feb 1, 2017

QuLogic Feb 1, 2017

madphysicist Feb 1, 2017

madphysicist commented Feb 1, 2017

madphysicist commented Feb 3, 2017

madphysicist commented Feb 3, 2017

madphysicist commented Feb 4, 2017

QuLogic Feb 5, 2017

story645 Feb 6, 2017

QuLogic Feb 6, 2017

madphysicist Feb 6, 2017

jenshnielsen Feb 6, 2017

QuLogic Feb 5, 2017

		@@ -536,12 +536,66 @@ def test_formatstrformatter():


		@pytest.mark.parametrize('xmax, decimals, symbol, x, display_range, expected',
		percentformatter_test_cases)

		@@ -511,35 +511,66 @@ def test_formatstrformatter():
		assert '002-01' == tmp_form(2, 1)


		percentformatter_test_cases = (

ENH: Fixed PercentFormatter usage with latex #7965

ENH: Fixed PercentFormatter usage with latex #7965

Conversation

madphysicist commented Jan 27, 2017 • edited Loading

madphysicist commented Jan 27, 2017

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

madphysicist commented Jan 30, 2017 • edited Loading

madphysicist commented Jan 30, 2017

story645 commented Jan 30, 2017

madphysicist commented Jan 30, 2017

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

madphysicist commented Feb 1, 2017

madphysicist commented Feb 3, 2017

madphysicist commented Feb 3, 2017

madphysicist commented Feb 4, 2017

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

madphysicist commented Jan 27, 2017 •

edited

Loading

madphysicist commented Jan 30, 2017 •

edited

Loading