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:  sage8.7 
Component:  packages: standard  Keywords:  upgrade, matplotlib 
Cc:  Antonio Rojas, François Bissey, Timo Kaufmann, Ximin Luo, KarlDieter 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: 
Description (last modified by )
This fixes compilation errors with clang 6.0.1
Tarball (created using spkgsrc): http://users.ox.ac.uk/~coml0531/sage/matplotlib2.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
Branch:  → u/dimpase/packages/matplotlib223 

Cc:  François Bissey added 
Commit:  → 72526fc9632cb7c2a643ef7215d1d3867b740605 
Status:  new → needs_review 
comment:2 Changed 4 years ago by
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
Milestone:  sage8.4 → sage8.5 

comment:4 Changed 4 years ago by
Cc:  KarlDieter Crisman added 

comment:5 Changed 4 years ago by
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
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
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 followup: 10 Changed 4 years ago by
sagemath depends on python 2 version of matplotlib (which was removed from debian unstable)
comment:9 Changed 4 years ago by
Milestone:  sage8.5 → sage8.7 

See also discussions on debiansciencesagemath.
comment:10 Changed 4 years ago by
Replying to chapoton:
sagemath depends on python 2 version of matplotlib (which was removed from debian unstable)
The python 2 version of matplotlib was just moved to a new package (matplotlib2), so it's fine. False alarm.
comment:12 Changed 4 years ago by
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
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
Authors:  Dima Pasechnik → Dima Pasechnik, Jeroen Demeyer 

Description:  modified (diff) 
Status:  needs_review → needs_work 
comment:15 followup: 18 Changed 4 years ago by
Status:  needs_work → needs_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
Branch:  u/dimpase/packages/matplotlib223 → u/jdemeyer/packages/matplotlib223 

comment:17 Changed 4 years ago by
Commit:  72526fc9632cb7c2a643ef7215d1d3867b740605 → 7794b098d90117eeeb07b9765bdb5985dada9134 

comment:18 Changed 4 years ago by
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
Description:  modified (diff) 

comment:20 Changed 4 years ago by
Reviewers:  → Dima Pasechnik 

Status:  needs_review → positive_review 
looks good.
comment:21 Changed 4 years ago by
Status:  positive_review → needs_work 

This needs more isscalar
fixes.
comment:22 Changed 4 years ago by
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
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/sitepackages/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
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
Commit:  7794b098d90117eeeb07b9765bdb5985dada9134 → c892da7dc21c63b3e5ee788fec7b10e8d1373820 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
c892da7  Avoid numpy.asscalar()

comment:26 Changed 4 years ago by
Status:  needs_work → needs_review 

comment:28 Changed 4 years ago by
Status:  needs_review → positive_review 

comment:29 Changed 4 years ago by
Branch:  u/jdemeyer/packages/matplotlib223 → c892da7dc21c63b3e5ee788fec7b10e8d1373820 

Resolution:  → fixed 
Status:  positive_review → closed 
New commits:
update matplotlib to version 2.2.3