GeezerVison

@mdboom While I realize that you have a script to do this, it is really disconcerting to see your name pop up fractions of a second after I create a PR :)

I am not familiar enough with Python 2.7 to immediately understand why the following is not working:

s = '{x:0.{decimals}f}'.format(x=x, decimals=decimals)

Are nested formats not allowed?

There are so many cleanup changes here that I recommend breaking this into two PRs: one for the cleanups, and a second one for the new feature.

No problem with that. Is there an easy way to do that without closing this PR?

@mdboom While I realize that you have a script to do this, it is really disconcerting to see your name pop up fractions of a second after I create a PR :)

That's https://waffle.io/

No problem with that. Is there an easy way to do that without closing this PR?

Create a new branch starting on this one, rebase it to include only cleanup, and open a new PR for it. When that's merged, rebase this one on top of master and drop all the cleanup from here.

I think you would make a separate branch on your repo for the cleanups, and make a PR from that. Then, on your pctFormatter branch, isolate the PercentFormatter part with additional commits, and use 'git rebase -i' to squash it down to a single commit. Force-push that to your github repo, and it will replace the present commits in the present PR.

I did the split (#6253) and fixed the Py2.7 error. Turns out Py3 automatically converts acceptable format precisions into an int. Py2 does not.

The only potentially unrelated change I kept here was moving __all__ to the top.

Metabolized the docs of EngFormatter.format_eng from #6253 into this commit to get rid of sphinx errors.

I think that this PR is complete (ready for further review). Same goes for #6253.

The failure in Appveyor is the sporadic image comparison failure. It is not related to the code I added as far as I can tell.

Squawk.

Cc: @efiring.

In reading the new code, I find that I waste a lot of time trying to figure out what d really is. I am comfortable with short variable names like x when their meaning is obvious from the context; but d is not obvious here--especially since some places it is in unscaled data units, and other places in percent. So maybe use dspan or xspan, and pcspan, or something like that? And maybe xmax instead of mx? Readability matters, both in the tests and in the active code.

I changed max to xmax throughout, and changed d to display_range and scaled_range as appropriate. I think that does make a big improvement to readability.

On a side-note, I got d from OldScalarFormatter, originally thinking to reuse its pprint method. I don't think that d needs to be changed at this point, but it is a possibility. d also appears in at least one of the Locators.

In addition to the pep-8 failure noted above there is a real failure that I have not tracked down (probably you will spot it instantly). Once those are taken care of, I think this will be functionally ready. It's a nice contribution. You could neaten it up by rebasing to squash the commits.

I went with

for case in test_cases:
    yield (_percent_format_helper, *case)

and also fixed the other stupid typo. Will push with squash soon.

And now I know why I should have listened the first time.

I don't think the Travis failure is related to my code.

Thank you, @madphysicist!

No problem. Thanks for looking over and accepting my PR!