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. |
lib/matplotlib/ticker.py
Outdated
|
|
||
| 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.
I think this should be == not is. 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 to
6d584ad
Compare
|
I rebased after deleting my last commit (the one with the enhancement #6533). Hopefully it will be OK. |
FIX: EngFormatter without but without prefix closes #6009
|
backported to v2.x as 59e0ab9 |
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”).