Opened 3 years ago
Closed 3 years ago
#26288 closed defect (fixed)
upgrade matplotlib to 2.2.3
Reported by:  dimpase  Owned by:  

Priority:  major  Milestone:  sage8.7 
Component:  packages: standard  Keywords:  upgrade, matplotlib 
Cc:  arojas, fbissey, ghtimokau, infinity0, kcrisman, saraedum, slelievre, thansen  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 3 years ago by
 Branch set to u/dimpase/packages/matplotlib223
 Cc fbissey added
 Commit set to 72526fc9632cb7c2a643ef7215d1d3867b740605
 Status changed from new to needs_review
comment:2 Changed 3 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 3 years ago by
 Milestone changed from sage8.4 to sage8.5
comment:4 Changed 3 years ago by
 Cc kcrisman added
comment:5 Changed 3 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 3 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 3 years ago by
 Cc arojas ghtimokau infinity0 saraedum slelievre thansen added
 Description modified (diff)
 Keywords upgrade matplotlib added
comment:8 followup: ↓ 10 Changed 3 years ago by
sagemath depends on python 2 version of matplotlib (which was removed from debian unstable)
comment:9 Changed 3 years ago by
 Milestone changed from sage8.5 to sage8.7
See also discussions on debiansciencesagemath.
comment:10 in reply to: ↑ 8 Changed 3 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:11 Changed 3 years ago by
ping?
comment:12 Changed 3 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 3 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 3 years ago by
 Description modified (diff)
 Status changed from needs_review to needs_work
comment:15 followup: ↓ 18 Changed 3 years ago by
 Status changed from needs_work to 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 3 years ago by
 Branch changed from u/dimpase/packages/matplotlib223 to u/jdemeyer/packages/matplotlib223
comment:17 Changed 3 years ago by
 Commit changed from 72526fc9632cb7c2a643ef7215d1d3867b740605 to 7794b098d90117eeeb07b9765bdb5985dada9134
comment:18 in reply to: ↑ 15 Changed 3 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 3 years ago by
 Description modified (diff)
comment:20 Changed 3 years ago by
 Reviewers set to Dima Pasechnik
 Status changed from needs_review to positive_review
looks good.
comment:21 Changed 3 years ago by
 Status changed from positive_review to needs_work
This needs more isscalar
fixes.
comment:22 Changed 3 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 3 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 3 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 3 years ago by
 Commit changed from 7794b098d90117eeeb07b9765bdb5985dada9134 to c892da7dc21c63b3e5ee788fec7b10e8d1373820
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
c892da7  Avoid numpy.asscalar()

comment:26 Changed 3 years ago by
 Status changed from needs_work to needs_review
comment:27 Changed 3 years ago by
OK, works, also with numpy from #27000
comment:28 Changed 3 years ago by
 Status changed from needs_review to positive_review
comment:29 Changed 3 years ago by
 Branch changed from u/jdemeyer/packages/matplotlib223 to c892da7dc21c63b3e5ee788fec7b10e8d1373820
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
update matplotlib to version 2.2.3