fix floating comparison in path.arc#26991
Conversation
|
Seems like all tests were OK? Maybe add a test like one of the examples in #26972 though? |
That's weird, numerous tests are failing on my machine mpl version is 3.9.0.dev356+g19c0e07558 |
|
Should the test be an image comparison test that with a baseline image? I haven't think about a way that we don't need to add a image |
|
Does the tests pass on main for you? A common reason for failing tests is font issues (one reason that MPL pin freetype to an old version is subtle differences in the exact rendering). A consequence is that it may be hard to generate test images on such an install (as they will maybe fail on the CI, but worth trying, it is actually possible to download the CI-generated image from the artifact and use that if it is a problem). Another reason is that a different processor architecture is used, which may lead to floating-point differences. Usually we compensate for that in the tests, e.g. matplotlib/lib/matplotlib/tests/test_contour.py Lines 384 to 387 in 1536245 But considering your multiple failures I'd guess freetype. |
Thanks for the help! There still test failures on main (I just assumed that they will work...) I setup the dev environment with conda following "Setting up Matplotlib for development", so I have no ida about where the font issues come from? |
|
My understanding of the problem is that:
I don't think that this is quite the right fix. The behavior we want is:
so I think the test is:
|
PR summary
Closes #26972. When drawing the spine for the radial axis, we call
Path.arcto draw. InPath.arc, we are using==for floating point comparison which may results in wrong results. The solution is to compare with a tolerance.Tests on
Path.arcis added to ensure that it's robust about the input change within [-1e-6, 1e-6]The solution is to replace this withnp.isclose()np.isclose(0, 1e-6)will give false, which is not we want.However, a large number of tests will break and I'm checking whether the difference of results will be noticed by human eyes.Failing locally, we think this is irrelevantPR checklist