Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Patch for issue #6009#6014
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Should add a unit test to exercise this. Preferably not an image test (we have plenty of those), but one that tests the strings coming out of EngFormatter. |
| @@ -960,7 +960,11 @@ def format_eng(self, num): | |||
| formatted = format_str % (mant, prefix) | |||
| return formatted.strip() | |||
| formatted = formatted.strip() | |||
| if (self.unit is not "") and (prefix is self.ENG_PREFIXES[0]): | |||
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.
I think this should be== notis. It is just a little less brittle.
Left minor comment, but other wise 👍 |
I don't understand why my last commit broke the Travis CI build test: it seems to me that the changes are really minor, aren't they? Furthermore, when looking at the failure logs in “Details” section of Travis, one can find errors like |
Travis is broken because the merge of another pr broke master is is not caused by your pr |
Ok, thank you for the explanation :). |
Can you rebase so we can merge it? |
I think I've done a mistake on this branch with my git local repo (what a change…). I used the same branch to implement the enhancement I presented in#6533 (which also fix this issue btw). Should I go back in time with git and then rebase, or do something else? I guess opening a new PR (on a branch from an up-to-date master) which fixes this small issue and also implements the |
Hope it is what was expected when asking for some unit (non image) test.
Sorry, I was using a different name than “mticker” to import the ticker module, and I had forgotten to change it when committing the new unit test test_EngFormatter_formatting()…
Now “All right” on pep8online.com for the method format_eng in the class EngFormatter.
5161bfa to6d584adCompareI rebased after deleting my last commit (the one with the enhancement#6533). Hopefully it will be OK. |
FIX: EngFormatter without but without prefixcloses#6009
tacaswell commentedJun 6, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
backported to v2.x as59e0ab9 |
Here is a patch for issue#6009 about the EngFormatter in matplotlib.ticker.
It simply enforces a space when there is no SI prefix but yet a unit symbol. For example “10 seconds” should be formatted as “10 s” by the EngFormatter (previously “10s”).