Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MNT: updates for python 3.7 #11533

Merged
merged 6 commits into from Aug 7, 2018
Merged

Conversation

tacaswell
Copy link
Member

Did not do appveyor as I am not sure if there is a miniconda available yet.

@tacaswell tacaswell added this to the v3.0 milestone Jun 28, 2018
@dopplershift
Copy link
Contributor

I don't think Travis has 3.7 yet. See travis-ci/travis-ci#9815.

@tacaswell tacaswell modified the milestones: v3.0, v2.2.3 Jun 28, 2018
@tacaswell
Copy link
Member Author

we probably want to backport all of this to 2.2.x as well.

@tacaswell
Copy link
Member Author

This are reproducible which is odd given that nightly is passing...

__________________________ test_contour_colorbar[svg] __________________________
[gw0] linux -- Python 3.7.0 /home/travis/virtualenv/python3.7.0/bin/python
expected = '/home/travis/build/matplotlib/matplotlib/result_images/test_axes/contour_colorbar-expected.svg'
actual = '/home/travis/build/matplotlib/matplotlib/result_images/test_axes/contour_colorbar.svg'
tol = 0
    def _raise_on_image_difference(expected, actual, tol):
        __tracebackhide__ = True
    
        err = compare_images(expected, actual, tol, in_decorator=True)
    
        if not os.path.exists(expected):
            raise ImageComparisonFailure('image does not exist: %s' % expected)
    
        if err:
            for key in ["actual", "expected"]:
                err[key] = os.path.relpath(err[key])
            raise ImageComparisonFailure(
                'images not close (RMS %(rms).3f):\n\t%(actual)s\n\t%(expected)s '
>                % err)
E           matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 0.002):
E           	result_images/test_axes/contour_colorbar_svg.png
E           	result_images/test_axes/contour_colorbar-expected_svg.png
lib/matplotlib/testing/decorators.py:148: ImageComparisonFailure
___________________________ test_tight_layout4[svg] ____________________________
[gw1] linux -- Python 3.7.0 /home/travis/virtualenv/python3.7.0/bin/python
expected = '/home/travis/build/matplotlib/matplotlib/result_images/test_tightlayout/tight_layout4-expected.svg'
actual = '/home/travis/build/matplotlib/matplotlib/result_images/test_tightlayout/tight_layout4.svg'
tol = 0
    def _raise_on_image_difference(expected, actual, tol):
        __tracebackhide__ = True
    
        err = compare_images(expected, actual, tol, in_decorator=True)
    
        if not os.path.exists(expected):
            raise ImageComparisonFailure('image does not exist: %s' % expected)
    
        if err:
            for key in ["actual", "expected"]:
                err[key] = os.path.relpath(err[key])
            raise ImageComparisonFailure(
                'images not close (RMS %(rms).3f):\n\t%(actual)s\n\t%(expected)s '
>                % err)
E           matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 0.006):
E           	result_images/test_tightlayout/tight_layout4_svg.png
E           	result_images/test_tightlayout/tight_layout4-expected_svg.png
lib/matplotlib/testing/decorators.py:148: ImageComparisonFailure

@tacaswell
Copy link
Member Author

Now just

___________________________ test_tight_layout4[svg] ____________________________
[gw1] linux -- Python 3.7.0 /home/travis/virtualenv/python3.7.0/bin/python
expected = '/home/travis/build/matplotlib/matplotlib/result_images/test_tightlayout/tight_layout4-expected.svg'
actual = '/home/travis/build/matplotlib/matplotlib/result_images/test_tightlayout/tight_layout4.svg'
tol = 0
    def _raise_on_image_difference(expected, actual, tol):
        __tracebackhide__ = True
    
        err = compare_images(expected, actual, tol, in_decorator=True)
    
        if not os.path.exists(expected):
            raise ImageComparisonFailure('image does not exist: %s' % expected)
    
        if err:
            for key in ["actual", "expected"]:
                err[key] = os.path.relpath(err[key])
            raise ImageComparisonFailure(
                'images not close (RMS %(rms).3f):\n\t%(actual)s\n\t%(expected)s '
>                % err)
E           matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 0.006):
E           	result_images/test_tightlayout/tight_layout4_svg.png
E           	result_images/test_tightlayout/tight_layout4-expected_svg.png
lib/matplotlib/testing/decorators.py:148: ImageComparisonFailure
=========================== short test summary info ============================

I can not reproduce this locally using 3.7.0 from anaconda.

@tacaswell tacaswell closed this Jul 15, 2018
@tacaswell tacaswell reopened this Jul 15, 2018
@tacaswell tacaswell added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Jul 15, 2018
 - run 3.7 with pep8
 - only run 1 3.5 build
 - bump ubuntu version, sudo seems to be required for 3.7
It seems that the paths output for glyphs in svg have changed (we are
outputting the text as paths not a strings) to now include a z (move
back to start to close the path) that the base line does not.  The
glyphs are closed paths so this should not matter, but the version of
inkscape on travis seems to be sensitive to it.
@tacaswell
Copy link
Member Author

@ImportanceOfBeingErnest Can you take a look at this? The test that is failing is

@check_figures_equal(extensions=["png", "pdf", "svg"])
def test_add_artist(fig_test, fig_ref):
fig_test.set_dpi(100)
fig_ref.set_dpi(100)
ax = fig_test.subplots()
l1 = plt.Line2D([.2, .7], [.7, .7])
l2 = plt.Line2D([.2, .7], [.8, .8])
r1 = plt.Circle((20, 20), 100, transform=None)
r2 = plt.Circle((.7, .5), .05)
r3 = plt.Circle((4.5, .8), .55, transform=fig_test.dpi_scale_trans,
facecolor='crimson')
for a in [l1, l2, r1, r2, r3]:
fig_test.add_artist(a)
l2.remove()
ax2 = fig_ref.subplots()
l1 = plt.Line2D([.2, .7], [.7, .7], transform=fig_ref.transFigure)
r1 = plt.Circle((20, 20), 100, transform=None, clip_on=False, zorder=20)
r2 = plt.Circle((.7, .5), .05, transform=fig_ref.transFigure)
r3 = plt.Circle((4.5, .8), .55, transform=fig_ref.dpi_scale_trans,
facecolor='crimson', clip_on=False, zorder=20)
for a in [l1, r1, r2, r3]:
ax2.add_artist(a)

which does not have an image we can re-generate like we discussed on the call.

Looking at the diff locally it looks like difference is the ordering of non-overlapping artists (before or after all of the bits of Axes).

The version of inkscape on the passing travis instances is 0.48, on the failing travis is 0.91 and on my local system is 0.92.

I strongly suspect this is a bug/instability in inkscape....

--- result_images/test_figure/test_add_artist-expected.svg      2018-08-04 23:59:15.377137248 -0400
+++ result_images/test_figure/test_add_artist.svg       2018-08-04 23:59:15.350471240 -0400
@@ -26,24 +26,6 @@
 z
 " style="fill:#ffffff;"/>
    </g>
-   <g id="C2">
-    <path clip-path="url(#p4c8e7e0101)" d="M 403.2 237.6 
-C 410.837849 237.6 418.1639 235.324088 423.564675 231.273506 
-C 428.96545 227.222925 432 221.728387 432 216 
-C 432 210.271613 428.96545 204.777075 423.564675 200.726494 
-C 418.1639 196.675912 410.837849 194.4 403.2 194.4 
-C 395.562151 194.4 388.2361 196.675912 382.835325 200.726494 
-C 377.43455 204.777075 374.4 210.271613 374.4 216 
-C 374.4 221.728387 377.43455 227.222925 382.835325 231.273506 
-C 388.2361 235.324088 395.562151 237.6 403.2 237.6 
-z
-" style="fill:#0000ff;stroke:#000000;stroke-linejoin:miter;"/>
-   </g>
-   <g id="l1">
-    <path clip-path="url(#p4c8e7e0101)" d="M 115.2 129.6 
-L 403.2 129.6 
-" style="fill:none;stroke:#0000ff;stroke-linecap:square;"/>
-   </g>
    <g id="patch_3">
     <path d="M 72 388.8 
 L 72 43.2 
@@ -490,8 +472,9 @@
      </g>
     </g>
    </g>
-   <g id="C1">
-    <path d="M 20 512 
+  </g>
+  <g id="C1">
+   <path d="M 20 512 
 C 46.52031 512 71.957987 501.463369 90.710678 482.710678 
 C 109.463369 463.957987 120 438.52031 120 412 
 C 120 385.47969 109.463369 360.042013 90.710678 341.289322 
@@ -502,9 +485,22 @@
 C -31.957987 501.463369 -6.52031 512 20 512 
 z
 " style="fill:#0000ff;stroke:#000000;stroke-linejoin:miter;"/>
-   </g>
-   <g id="C3">
-    <path d="M 324 414 
+  </g>
+  <g id="C2">
+   <path d="M 403.2 237.6 
+C 410.837849 237.6 418.1639 235.324088 423.564675 231.273506 
+C 428.96545 227.222925 432 221.728387 432 216 
+C 432 210.271613 428.96545 204.777075 423.564675 200.726494 
+C 418.1639 196.675912 410.837849 194.4 403.2 194.4 
+C 395.562151 194.4 388.2361 196.675912 382.835325 200.726494 
+C 377.43455 204.777075 374.4 210.271613 374.4 216 
+C 374.4 221.728387 377.43455 227.222925 382.835325 231.273506 
+C 388.2361 235.324088 395.562151 237.6 403.2 237.6 
+z
+" style="fill:#0000ff;stroke:#000000;stroke-linejoin:miter;"/>
+  </g>
+  <g id="C3">
+   <path d="M 324 414 
 C 334.502043 414 344.575363 409.827494 352.001429 402.401429 
 C 359.427494 394.975363 363.6 384.902043 363.6 374.4 
 C 363.6 363.897957 359.427494 353.824637 352.001429 346.398571 
@@ -515,12 +511,11 @@
 C 303.424637 409.827494 313.497957 414 324 414 
 z
 " style="fill:#dc143c;stroke:#000000;stroke-linejoin:miter;"/>
-   </g>
+  </g>
+  <g id="l1">
+   <path d="M 115.2 129.6 
+L 403.2 129.6 
+" style="fill:none;stroke:#0000ff;stroke-linecap:square;"/>
   </g>
  </g>
- <defs>
-  <clipPath id="p4c8e7e0101">
-   <rect height="345.6" width="446.4" x="72" y="43.2"/>
-  </clipPath>
- </defs>
 </svg>

@ImportanceOfBeingErnest
Copy link
Member

@tacaswell I'm not sure I understand this. If you see a difference in the generated svg, the issue can't be due to inkscape because inkscape is not used to generate the svg. Rather, the svg backend generates the svg and I can actually well imagine that there is a difference between the two figures to compare here due to the order of adding elements to the xml file.

If then inkscape renders the svg as png, it might render different svg input differently, maybe it just worked by coincidence in 0.48. So I think the test is not well designed to be compared via svg.

The purpose of this test is actually to compare positions of elements, I did not even think about zorder. Also I have to mention I only included svg to the tests, simply because it worked.

Thinking about it, one reason to use fig.add_artist(someartist) instead of ax.add_artist(someartist(...,transform=fig.transFigure)) might actually precisely be that the zorder in the resulting figure is different.
Due to this I would propose to leave out svg testing for this one completely and if possible return to zero tollerance.

Once there is travis testing for 3.7 available (through this PR, I suppose?) I could see if there is a better test case to test zordering (since it's hard to do locally with all the different versions in use).

@tacaswell
Copy link
Member Author

The relative z-order of non-overlapping artists should not matter, I think this is a perfectly valid test. The reason we do the svg testing through inkscape rather than direct text comparison is that we want to be insensitive to permutations like this. If inkscape is rendering different raster output for swapping around the definition order of non-overlapping vectors, then I think that is a bug in inkscape.

Tweaking the test a bit more we can get the diff down too:

--- result_images/test_figure/test_add_artist-expected.svg      2018-08-05 17:44:11.678082858 -0400
+++ result_images/test_figure/test_add_artist.svg       2018-08-05 17:44:11.658083381 -0400
@@ -472,8 +472,9 @@
      </g>
     </g>
    </g>
-   <g id="C1">
-    <path d="M 20 512 
+  </g>
+  <g id="C1">
+   <path d="M 20 512 
 C 46.52031 512 71.957987 501.463369 90.710678 482.710678 
 C 109.463369 463.957987 120 438.52031 120 412 
 C 120 385.47969 109.463369 360.042013 90.710678 341.289322 
@@ -484,9 +485,9 @@
 C -31.957987 501.463369 -6.52031 512 20 512 
 z
 " style="fill:#0000ff;stroke:#000000;stroke-linejoin:miter;"/>
-   </g>
-   <g id="C2">
-    <path clip-path="url(#p4c8e7e0101)" d="M 403.2 237.6 
+  </g>
+  <g id="C2">
+   <path d="M 403.2 237.6 
 C 410.837849 237.6 418.1639 235.324088 423.564675 231.273506 
 C 428.96545 227.222925 432 221.728387 432 216 
 C 432 210.271613 428.96545 204.777075 423.564675 200.726494 
@@ -497,9 +498,9 @@
 C 388.2361 235.324088 395.562151 237.6 403.2 237.6 
 z
 " style="fill:#0000ff;stroke:#000000;stroke-linejoin:miter;"/>
-   </g>
-   <g id="C3">
-    <path d="M 324 414 
+  </g>
+  <g id="C3">
+   <path d="M 324 414 
 C 334.502043 414 344.575363 409.827494 352.001429 402.401429 
 C 359.427494 394.975363 363.6 384.902043 363.6 374.4 
 C 363.6 363.897957 359.427494 353.824637 352.001429 346.398571 
@@ -510,17 +511,11 @@
 C 303.424637 409.827494 313.497957 414 324 414 
 z
 " style="fill:#dc143c;stroke:#000000;stroke-linejoin:miter;"/>
-   </g>
-   <g id="l1">
-    <path clip-path="url(#p4c8e7e0101)" d="M 115.2 129.6 
+  </g>
+  <g id="l1">
+   <path d="M 115.2 129.6 
 L 403.2 129.6 
 " style="fill:none;stroke:#0000ff;stroke-linecap:square;"/>
-   </g>
   </g>
  </g>
- <defs>
-  <clipPath id="p4c8e7e0101">
-   <rect height="345.6" width="446.4" x="72" y="43.2"/>
-  </clipPath>
- </defs>
 </svg>

which is:

  • indentation differences because the artists are at different scopes
  • an extra clip path (that the artists do not intersect with)

Lets see if that passes....

 - add gid to artists to make SVG more readable
 - make sure zorder is identical

This makes the diff between the expected and produced svgs readable.

The svg tests passes locally (inkscape 0.92) and on other travis
runs (inkscape 0.48).  Fails on travis with inkscape 0.91
These seem to fail on the new travis ubuntu image, may be inkscape
version related.
@tacaswell tacaswell merged commit 0e95b73 into matplotlib:master Aug 7, 2018
@tacaswell tacaswell deleted the update_for_py37 branch August 7, 2018 01:33
@tacaswell
Copy link
Member Author

self-merged based on reviews from @efiring and @dopplershift

@lumberbot-app
Copy link

lumberbot-app bot commented Aug 7, 2018

There seem to be a conflict, please backport manually

tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Aug 7, 2018
MNT: updates for python 3.7
Conflicts:
	.travis.yml
          - mostly keep 2.2.x version, but add py37 section
	INSTALL.rst
          - kept v2.2.x wording
	lib/matplotlib/tests/test_figure.py
          - removed non-existent on 2.2.x test (feature tested
            is only on master)
tacaswell added a commit that referenced this pull request Aug 8, 2018
Merge pull request #11533 from tacaswell/update_for_py37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants