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: sage-8.7
Component: packages: standard Keywords: upgrade, matplotlib
Cc: arojas, fbissey, gh-timokau, 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:

Status badges

Description (last modified by jdemeyer)

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 3 years ago by dimpase

  • Branch set to u/dimpase/packages/matplotlib223
  • Cc fbissey added
  • Commit set to 72526fc9632cb7c2a643ef7215d1d3867b740605
  • Status changed from new to needs_review

New commits:

72526fcupdate matplotlib to version 2.2.3

comment:2 Changed 3 years ago by Konrad127123

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 dimpase

  • Milestone changed from sage-8.4 to sage-8.5

comment:4 Changed 3 years ago by tscrim

  • Cc kcrisman added

comment:5 Changed 3 years ago by dimpase

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 fbissey

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 slelievre

  • Cc arojas gh-timokau infinity0 saraedum slelievre thansen added
  • Description modified (diff)
  • Keywords upgrade matplotlib added

comment:8 follow-up: Changed 3 years ago by 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 3 years ago by slelievre

  • Milestone changed from sage-8.5 to sage-8.7

comment:10 in reply to: ↑ 8 Changed 3 years ago by thansen

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 3 years ago by gh-dimpase

ping?

comment:12 Changed 3 years ago by jdemeyer

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 jdemeyer

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 jdemeyer

  • Authors changed from Dima Pasechnik to Dima Pasechnik, Jeroen Demeyer
  • Description modified (diff)
  • Status changed from needs_review to needs_work

comment:15 follow-up: Changed 3 years ago by dimpase

  • 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 jdemeyer

  • Branch changed from u/dimpase/packages/matplotlib223 to u/jdemeyer/packages/matplotlib223

comment:17 Changed 3 years ago by jdemeyer

  • Commit changed from 72526fc9632cb7c2a643ef7215d1d3867b740605 to 7794b098d90117eeeb07b9765bdb5985dada9134

New commits:

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

comment:18 in reply to: ↑ 15 Changed 3 years ago by jdemeyer

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 jdemeyer

  • Description modified (diff)

comment:20 Changed 3 years ago by dimpase

  • Reviewers set to Dima Pasechnik
  • Status changed from needs_review to positive_review

looks good.

comment:21 Changed 3 years ago by jdemeyer

  • Status changed from positive_review to needs_work

This needs more isscalar fixes.

comment:22 Changed 3 years ago by dimpase

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 dimpase

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 3 years ago by jdemeyer

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 git

  • Commit changed from 7794b098d90117eeeb07b9765bdb5985dada9134 to c892da7dc21c63b3e5ee788fec7b10e8d1373820

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

c892da7Avoid numpy.asscalar()

comment:26 Changed 3 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:27 Changed 3 years ago by dimpase

OK, works, also with numpy from #27000

comment:28 Changed 3 years ago by dimpase

  • Status changed from needs_review to positive_review

comment:29 Changed 3 years ago by vbraun

  • Branch changed from u/jdemeyer/packages/matplotlib223 to c892da7dc21c63b3e5ee788fec7b10e8d1373820
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.