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

Update matplotlib to 3.3 #30176

Closed
antonio-rojas opened this issue Jul 19, 2020 · 52 comments
Closed

Update matplotlib to 3.3 #30176

antonio-rojas opened this issue Jul 19, 2020 · 52 comments

Comments

@antonio-rojas
Copy link
Contributor

Brings lots of deprecation warnings on every plot() call, due to OldScalarFormatter deprecation.

Depends on pillow for all image formats now.

Upstream: Fixed upstream, but not in a stable release.

CC: @jhpalmieri @kiwifb @mkoeppe @timokau @egourgoulhon @antonio-rojas @zlscherr

Component: packages: standard

Author: Antonio Rojas

Branch: 73af14a

Reviewer: Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/30176

@antonio-rojas antonio-rojas added this to the sage-9.2 milestone Jul 19, 2020
@antonio-rojas
Copy link
Contributor Author

Branch: u/arojas/update_matplotlib_to_3_3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 19, 2020

Commit: cdf20ea

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 19, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

cdf20eaReplace deprecated OldScalarFormatter with ScalarFormatter

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 19, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

fee97c7Replace deprecated basex and basey parameters
3d41b34Update example
80f12faUpdate metrics for matplotlib 3.3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 19, 2020

Changed commit from cdf20ea to 80f12fa

@antonio-rojas
Copy link
Contributor Author

Author: Antonio Rojas

@antonio-rojas

This comment has been minimized.

@antonio-rojas
Copy link
Contributor Author

comment:5

Still 3 test failures, all looking like

File "/usr/lib/python3.8/site-packages/sage/geometry/polyhedron/plot.py", line 1030, in sage.geometry.polyhedron.plot.Projection.render_2d
Failed example:
    q1.plot() + q2.plot() + q3.plot() + q4.plot()
Expected:
    Graphics object consisting of 17 graphics primitives
Got:
    doctest:warning
      File "/usr/bin/sage-runtests", line 177, in <module>
        err = DC.run()
      File "/usr/lib/python3.8/site-packages/sage/doctest/control.py", line 1196, in run
        self.run_doctests()
      File "/usr/lib/python3.8/site-packages/sage/doctest/control.py", line 897, in run_doctests
        self.dispatcher.dispatch()
      File "/usr/lib/python3.8/site-packages/sage/doctest/forker.py", line 2038, in dispatch
        self.parallel_dispatch()
      File "/usr/lib/python3.8/site-packages/sage/doctest/forker.py", line 1933, in parallel_dispatch
        w.start()  # This might take some time
      File "/usr/lib/python3.8/site-packages/sage/doctest/forker.py", line 2205, in start
        super(DocTestWorker, self).start()
      File "/usr/lib/python3.8/multiprocessing/process.py", line 121, in start
        self._popen = self._Popen(self)
      File "/usr/lib/python3.8/multiprocessing/context.py", line 224, in _Popen
        return _default_context.get_context().Process._Popen(process_obj)
      File "/usr/lib/python3.8/multiprocessing/context.py", line 277, in _Popen
        return Popen(process_obj)
      File "/usr/lib/python3.8/multiprocessing/popen_fork.py", line 19, in __init__
        self._launch(process_obj)
      File "/usr/lib/python3.8/multiprocessing/popen_fork.py", line 75, in _launch
        code = process_obj._bootstrap(parent_sentinel=child_r)
      File "/usr/lib/python3.8/multiprocessing/process.py", line 315, in _bootstrap
        self.run()
      File "/usr/lib/python3.8/site-packages/sage/doctest/forker.py", line 2177, in run
        task(self.options, self.outtmpfile, msgpipe, self.result_queue)
      File "/usr/lib/python3.8/site-packages/sage/doctest/forker.py", line 2506, in __call__
        doctests, extras = self._run(runner, options, results)
      File "/usr/lib/python3.8/site-packages/sage/doctest/forker.py", line 2552, in _run
        result = runner.run(test)
      File "/usr/lib/python3.8/site-packages/sage/doctest/forker.py", line 905, in run
        return self._run(test, compileflags, out)
      File "/usr/lib/python3.8/site-packages/sage/doctest/forker.py", line 707, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/lib/python3.8/site-packages/sage/doctest/forker.py", line 1131, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.geometry.polyhedron.plot.Projection.render_2d[8]>", line 1, in <module>
        q1.plot() + q2.plot() + q3.plot() + q4.plot()
      File "/usr/lib/python3.8/site-packages/sage/repl/rich_output/display_manager.py", line 811, in displayhook
        plain_text, rich_output = self._rich_output_formatter(obj, dict())
      File "/usr/lib/python3.8/site-packages/sage/repl/rich_output/display_manager.py", line 625, in _rich_output_formatter
        rich_output = self._call_rich_repr(obj, rich_repr_kwds)
      File "/usr/lib/python3.8/site-packages/sage/repl/rich_output/display_manager.py", line 590, in _call_rich_repr
        warnings.warn(
      File "/usr/lib/python3.8/warnings.py", line 109, in _showwarnmsg
        sw(msg.message, msg.category, msg.filename, msg.lineno,
    :
    sage.repl.rich_output.display_manager.RichReprWarning: Exception in _rich_repr_ while displaying object: The first element of 'code' must be equal to 'MOVETO' (1).  Your first code is 79
    Graphics object consisting of 17 graphics primitives
**********************************************************************

@antonio-rojas
Copy link
Contributor Author

Upstream: Reported upstream. No feedback yet.

@antonio-rojas
Copy link
Contributor Author

comment:6

Reported upstream: matplotlib/matplotlib#17975

@mkoeppe
Copy link
Member

mkoeppe commented Jul 22, 2020

comment:7

Upstream seems to have addressed it in matplotlib/matplotlib#17982

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 22, 2020

Changed commit from 80f12fa to dbdecd9

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 22, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

dbdecd9Backport patch to fix the path of degenerate polygons

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 22, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

fd2a9b8Bump version to account for the added patch

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 22, 2020

Changed commit from dbdecd9 to fd2a9b8

@antonio-rojas
Copy link
Contributor Author

comment:10

No more test failures on Arch -> needs review

@antonio-rojas
Copy link
Contributor Author

Changed upstream from Reported upstream. No feedback yet. to Fixed upstream, but not in a stable release.

@mkoeppe
Copy link
Member

mkoeppe commented Jul 22, 2020

comment:11

Tests run at https://github.com/mkoeppe/sage/actions/runs/178114621

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 22, 2020

Changed commit from fd2a9b8 to ad67266

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 22, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

ad67266Remove merged patch

@mkoeppe
Copy link
Member

mkoeppe commented Jul 23, 2020

comment:13

Tests run at https://github.com/mkoeppe/sage/actions/runs/179391787

@mkoeppe
Copy link
Member

mkoeppe commented Jul 24, 2020

comment:14

Fails on all platforms. For example https://github.com/mkoeppe/sage/runs/901333269

    error: Failed to download FreeType. Please download one of ['https://downloads.sourceforge.net/project/freetype/freetype2/2.6.1/freetype-2.6.1.tar.gz', 'https://download.savannah.gnu.org/releases/freetype/freetype-2.6.1.tar.gz'] and extract it into build/freetype-2.6.1 at the top-level of the source repository.
    Running setup.py install for matplotlib: finished with status 'error'
ERROR: Command errored out with exit status 1: /sage/local/bin/python3 -u -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'/tmp/pip-req-build-_2y953_j/setup.py'"'"'; __file__='"'"'/tmp/pip-req-build-_2y953_j/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' --no-user-cfg install --record /tmp/pip-record-v8gxera1/install-record.txt --single-version-externally-managed --root /sage/local/var/tmp/sage/build/matplotlib-3.3.0.p0/inst --compile --install-headers /sage/local/var/tmp/sage/build/matplotlib-3.3.0.p0/inst/sage/local/include/python3.7m/matplotlib Check the logs for full command output.
Exception information:
Traceback (most recent call last):
  File "/sage/local/lib/python3.7/site-packages/pip/_internal/req/req_install.py", line 841, in install
    req_description=str(self.req),
  File "/sage/local/lib/python3.7/site-packages/pip/_internal/operations/install/legacy.py", line 86, in install
    raise LegacyInstallFailure

@mkoeppe
Copy link
Member

mkoeppe commented Jul 24, 2020

comment:16

https://github.com/matplotlib/matplotlib/blob/v3.3.x/INSTALL.rst:
"By default ... Matplotlib downloads and builds its own copy of FreeType ... and uses its own copy of Qhull.
To force Matplotlib to use a copy of FreeType or Qhull already installed in your system, create a setup.cfg file ...."

@mkoeppe
Copy link
Member

mkoeppe commented Jul 25, 2020

Reviewer: Matthias Koeppe

@mkoeppe
Copy link
Member

mkoeppe commented Jul 25, 2020

comment:31

Replying to @mkoeppe:

Tests run at https://github.com/mkoeppe/sage/actions/runs/181664763

This looks clean on all platforms.

I think it would be better if the changed multigraphics doctest was adjusted to be more tolerant instead of just updating the result.

Nevertheless, a positive review from my side.

@vbraun
Copy link
Member

vbraun commented Jul 27, 2020

comment:32

There is numerical noise on various buildbots, e.g.

**********************************************************************
File "src/sage/plot/multigraphics.py", line 1297, in sage.plot.multigraphics.GraphicsArray.position
Failed example:
    G.position(0)  # tol 1.0e-13
Expected:
    (0.025045451349937315,
     0.03415488992713045,
     0.4489880779745068,
     0.9345951100728696)
Got:
    (0.02494779509993732,
     0.03415488992713045,
     0.44913456234950677,
     0.9345951100728696)
Tolerance exceeded in 2 of 4:
    0.025045451349937315 vs 0.02494779509993732, tolerance 4e-3 > 1e-13
    0.4489880779745068 vs 0.44913456234950677, tolerance 4e-4 > 1e-13
**********************************************************************
File "src/sage/plot/multigraphics.py", line 1302, in sage.plot.multigraphics.GraphicsArray.position
Failed example:
    G.position(1)  # tol 1.0e-13
Expected:
    (0.5170637412999687,
     0.20212705964722733,
     0.4489880779745068,
     0.5986507706326758)
Got:
    (0.5170149131749686,
     0.20202940339722741,
     0.44913456234950666,
     0.5988460831326756)
Tolerance exceeded in 4 of 4:
    0.5170637412999687 vs 0.5170149131749686, tolerance 1e-4 > 1e-13
    0.20212705964722733 vs 0.20202940339722741, tolerance 5e-4 > 1e-13
    0.4489880779745068 vs 0.44913456234950666, tolerance 4e-4 > 1e-13
    0.5986507706326758 vs 0.5988460831326756, tolerance 4e-4 > 1e-13
**********************************************************************
1 item had failures:
   2 of   6 in sage.plot.multigraphics.GraphicsArray.position
    [192 tests, 2 failures, 20.75 s]
----------------------------------------------------------------------
sage -t --long --random-seed=0 src/sage/plot/multigraphics.py  # 2 doctests failed
----------------------------------------------------------------------

@vbraun
Copy link
Member

vbraun commented Jul 27, 2020

comment:33
**********************************************************************
File "src/sage/plot/multigraphics.py", line 1297, in sage.plot.multigraphics.GraphicsArray.position
Failed example:
    G.position(0)  # tol 1.0e-13
Expected:
    (0.025045451349937315,
     0.03415488992713045,
     0.4489880779745068,
     0.9345951100728696)
Got:
    (0.02494779509993732,
     0.03415488992713045,
     0.44913456234950677,
     0.9345951100728696)
Tolerance exceeded in 2 of 4:
    0.025045451349937315 vs 0.02494779509993732, tolerance 4e-3 > 1e-13
    0.4489880779745068 vs 0.44913456234950677, tolerance 4e-4 > 1e-13
**********************************************************************
File "src/sage/plot/multigraphics.py", line 1302, in sage.plot.multigraphics.GraphicsArray.position
Failed example:
    G.position(1)  # tol 1.0e-13
Expected:
    (0.5170637412999687,
     0.20212705964722733,
     0.4489880779745068,
     0.5986507706326758)
Got:
    (0.5170149131749686,
     0.20202940339722741,
     0.44913456234950666,
     0.5988460831326756)
Tolerance exceeded in 4 of 4:
    0.5170637412999687 vs 0.5170149131749686, tolerance 1e-4 > 1e-13
    0.20212705964722733 vs 0.20202940339722741, tolerance 5e-4 > 1e-13
    0.4489880779745068 vs 0.44913456234950666, tolerance 4e-4 > 1e-13
    0.5986507706326758 vs 0.5988460831326756, tolerance 4e-4 > 1e-13
**********************************************************************
1 item had failures:
   2 of   6 in sage.plot.multigraphics.GraphicsArray.position
    [192 tests, 2 failures, 27.84 s]
----------------------------------------------------------------------
sage -t --long --random-seed=0 src/sage/plot/multigraphics.py  # 2 doctests failed
----------------------------------------------------------------------

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 1, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

20ecd18Increase numerical noise tolerance

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 1, 2020

Changed commit from 79249f7 to 20ecd18

@vbraun
Copy link
Member

vbraun commented Aug 6, 2020

comment:37
**********************************************************************
File "src/sage/plot/multigraphics.py", line 1297, in sage.plot.multigraphics.GraphicsArray.position
Failed example:
    G.position(0)  # tol 1.0e-3
Expected:
    (0.025045451349937315,
     0.03415488992713045,
     0.4489880779745068,
     0.9345951100728696)
Got:
    (0.02494779509993732,
     0.03415488992713045,
     0.44913456234950677,
     0.9345951100728696)
Tolerance exceeded in 1 of 4:
    0.025045451349937315 vs 0.02494779509993732, tolerance 4e-3 > 1e-3
**********************************************************************
1 item had failures:
   1 of   6 in sage.plot.multigraphics.GraphicsArray.position
    [192 tests, 1 failure, 33.68 s]
----------------------------------------------------------------------
sage -t --long --random-seed=0 src/sage/plot/multigraphics.py  # 1 doctest failed
----------------------------------------------------------------------

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 7, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

73af14aAdjust numeric noise tolerance further

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 7, 2020

Changed commit from 20ecd18 to 73af14a

@vbraun
Copy link
Member

vbraun commented Aug 9, 2020

Changed branch from u/arojas/update_matplotlib_to_3_3 to 73af14a

@kcrisman
Copy link
Member

Changed commit from 73af14a to none

@kcrisman
Copy link
Member

comment:41

Hi! Is it possible that this change is responsible for the difference between the following two images?

Notice that not only is the image slightly narrower, but the labels now all have default of one place beyond the decimal point, which in the past was not desired for the evident reasons we see here. That might be good for scientific plotting, but not for mathematical plotting.

@kcrisman
Copy link
Member

comment:42

(Note that this explanation of the difference doesn't really seem to have any examples relevant to this situation.)

@mkoeppe
Copy link
Member

mkoeppe commented Jan 2, 2021

comment:43

Follow-up on qhull in #31148

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants