Make rounding behavior consistent #7573
Conversation
There's absolutely no reason to have our own implementation of this.
|
I'm fine with the change, but this is really not release critical and should really not delay the release (which is what release critical means). |
|
I am a bit disappointed that this does not break any tests. Removing |
|
I was pretty flabbergasted about the lack of any test failures here. I have a case I can add for the agg renderer's I'll add an API change note. |
|
I wonder if the tests that should be failing are the ones that have slightly higher tolerances now? |
|
The two tests for which I raised the tolerance were failing only on Windows, and their tolerances are nonzero only on Windows. |
|
It was only for dpi in the ~300 range for me to get images that actually showed a difference, at least for my test. Also, don't most of our image tests turn off the text for better consistency. It's possible that's not nearly so necessary with this change. |
| @@ -27,7 +27,7 @@ def __init__(self, dates, fmt='%Y-%m-%d'): | |||
|
|
|||
| def __call__(self, x, pos=0): | |||
| 'Return the label for time x at position pos' | |||
| ind = int(round(x)) | |||
There was a problem hiding this comment.
Do you mind if I push to your branch to change the import of numpy to follow the standards?
|
LGTM 👍 |
The behavior of the builtin round() function changed between python 2 and 3. Use numpy's, which matches python 3's, so that we have consistent rounding behavior between python versions. This helps keep image test results (here or downstream) consistent between python versions.
2bb6239 to
a7e8922
Compare
|
@NelleV I went ahead and updated the numpy import in that example. |
The behavior of the builtin
round()function changed between python 2 and 3. There are a few places in our code base that use it, resulting in occasional pixel jumps in things like text positioning; this makes image tests have needless noise. The easy solution is to use numpy's round everywhere to give us consistent behavior (and behavior which matches python 3's.) It looks like this had been done in some places, but not these last few.I also removed an implementation of
round()in the TkAgg backend which was unused. Some may consider this an API break, so I'm open to eliminating this commit or making TkAgg's an alias to numpy's.