Opened 4 years ago

Closed 4 years ago

#26288 closed defect (fixed)

upgrade matplotlib to 2.2.3

Reported by: Dima Pasechnik Owned by:
Priority: major Milestone: sage-8.7
Component: packages: standard Keywords: upgrade, matplotlib
Cc: Antonio Rojas, François Bissey, Timo Kaufmann, Ximin Luo, Karl-Dieter Crisman, Julian Rüth, Samuel Lelièvre, Tobias Hansen Merged in:
Authors: Dima Pasechnik, Jeroen Demeyer Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: c892da7 (Commits, GitHub, GitLab) Commit: c892da7dc21c63b3e5ee788fec7b10e8d1373820
Dependencies: Stopgaps:

Status badges

Description (last modified by Jeroen Demeyer)

This fixes compilation errors with clang 6.0.1

Tarball (created using spkg-src): http://users.ox.ac.uk/~coml0531/sage/matplotlib-2.2.3.tar.bz2

We also add a patch (taken from upstream) to fix deprecation warnings with recent numpy versions. This is needed for #27000.

Our last upgrade was to matplotlib 2.2.2 in #25702.

Note that matplotlib 3.x.y versions require Python 3.

Change History (29)

comment:1 Changed 4 years ago by Dima Pasechnik

Branch: u/dimpase/packages/matplotlib223
Cc: François Bissey added
Commit: 72526fc9632cb7c2a643ef7215d1d3867b740605
Status: newneeds_review

New commits:

72526fcupdate matplotlib to version 2.2.3

comment:2 Changed 4 years ago by Konrad Dabrowski

Building cvxopt with SAGE_CHECK="yes" still fails when running headless (#24657). Not sure if that's strictly a cvxopt or a matplotlib issue.

comment:3 Changed 4 years ago by Dima Pasechnik

Milestone: sage-8.4sage-8.5

comment:4 Changed 4 years ago by Travis Scrimshaw

Cc: Karl-Dieter Crisman added

comment:5 Changed 4 years ago by Dima Pasechnik

I don't think cvxopt (which needs to be updated) has to be dealt with here. Can we proceed with this update?

comment:6 Changed 4 years ago by François Bissey

I expressed my opinion on cvxopt so called test failures in #24657. I heard nothing back. And I definitely think we should proceed.

comment:7 Changed 4 years ago by Samuel Lelièvre

Cc: Antonio Rojas Timo Kaufmann Ximin Luo Julian Rüth Samuel Lelièvre Tobias Hansen added
Description: modified (diff)
Keywords: upgrade matplotlib added

comment:8 Changed 4 years ago by Frédéric Chapoton

sagemath depends on python 2 version of matplotlib (which was removed from debian unstable)

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=917428

comment:9 Changed 4 years ago by Samuel Lelièvre

Milestone: sage-8.5sage-8.7

comment:10 in reply to:  8 Changed 4 years ago by Tobias Hansen

Replying to chapoton:

sagemath depends on python 2 version of matplotlib (which was removed from debian unstable)

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=917428

The python 2 version of matplotlib was just moved to a new package (matplotlib2), so it's fine. False alarm.

comment:11 Changed 4 years ago by Dima Pasechnik

ping?

comment:12 Changed 4 years ago by Jeroen Demeyer

I would prefer to add the patches removing np.isscalar which will otherwise cause breakage with the numpy upgrade.

Can you look into that or should I?

comment:13 Changed 4 years ago by Jeroen Demeyer

There is this matplotlib PR, but I don't know if it is sufficient: https://github.com/matplotlib/matplotlib/pull/12070

comment:14 Changed 4 years ago by Jeroen Demeyer

Authors: Dima PasechnikDima Pasechnik, Jeroen Demeyer
Description: modified (diff)
Status: needs_reviewneeds_work

comment:15 Changed 4 years ago by Dima Pasechnik

Status: needs_workneeds_review

I don't understand what numpy upgrade that might cause anything to matplotlib is meant. Things still seem to work here with Sage 8.7.beta1.

I don't think it's a good idea to try to backport stuff just to hopefully fix bugs we don't see yet.

comment:16 Changed 4 years ago by Jeroen Demeyer

Branch: u/dimpase/packages/matplotlib223u/jdemeyer/packages/matplotlib223

comment:17 Changed 4 years ago by Jeroen Demeyer

Commit: 72526fc9632cb7c2a643ef7215d1d3867b7406057794b098d90117eeeb07b9765bdb5985dada9134

New commits:

c009e77update matplotlib to version 2.2.3
7794b09Avoid numpy.isscalar()

comment:18 in reply to:  15 Changed 4 years ago by Jeroen Demeyer

Replying to dimpase:

I don't understand what numpy upgrade that might cause anything to matplotlib is meant.

I mean the numpy upgrade to 1.16.0 at #27000.

I don't think it's a good idea to try to backport stuff just to hopefully fix bugs we don't see yet.

Why not? If this issue is blocking #27000, it should be addressed.

comment:19 Changed 4 years ago by Jeroen Demeyer

Description: modified (diff)

comment:20 Changed 4 years ago by Dima Pasechnik

Reviewers: Dima Pasechnik
Status: needs_reviewpositive_review

looks good.

comment:21 Changed 4 years ago by Jeroen Demeyer

Status: positive_reviewneeds_work

This needs more isscalar fixes.

comment:22 Changed 4 years ago by Dima Pasechnik

Do you mean np.asscalar ? Indeed, with numpy 1.16 I see

    DeprecationWarning: np.asscalar(a) is deprecated since NumPy v1.16, use a.item() instead

comment:23 Changed 4 years ago by Dima Pasechnik

In fact, I don't get why you need to do anything with isscalar:

sage: import numpy as np
sage: np.version.version
'1.16.0'
sage: np.isscalar(1)
True

The problem is in asscalar!

sage: import numpy as np
sage: np.version.version
'1.16.0'
sage: np.asscalar(np.array([24]))
/home/scratch2/dimpase/sage/sage/local/lib/python2.7/site-packages/numpy/lib/type_check.py:546: DeprecationWarning: np.asscalar(a) is deprecated since NumPy v1.16, use a.item() instead
  'a.item() instead', DeprecationWarning, stacklevel=1)
24

comment:24 Changed 4 years ago by Jeroen Demeyer

isscalar vs asscalar: it's just a 1 bit difference. Somebody in matplotlib didn't pay attention during their coding theory course :-)

The relevant PR is https://github.com/matplotlib/matplotlib/pull/12508

comment:25 Changed 4 years ago by git

Commit: 7794b098d90117eeeb07b9765bdb5985dada9134c892da7dc21c63b3e5ee788fec7b10e8d1373820

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

c892da7Avoid numpy.asscalar()

comment:26 Changed 4 years ago by Jeroen Demeyer

Status: needs_workneeds_review

comment:27 Changed 4 years ago by Dima Pasechnik

OK, works, also with numpy from #27000

comment:28 Changed 4 years ago by Dima Pasechnik

Status: needs_reviewpositive_review

comment:29 Changed 4 years ago by Volker Braun

Branch: u/jdemeyer/packages/matplotlib223c892da7dc21c63b3e5ee788fec7b10e8d1373820
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.