Opened 4 years ago

Closed 4 years ago

#23711 closed enhancement (fixed)

sage-env: do not depend on matplotlib version number

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-8.1
Component: scripts Keywords:
Cc: embray Merged in:
Authors: Jeroen Demeyer Reviewers: Erik Bray, Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: e3c1b2b (Commits, GitHub, GitLab) Commit: e3c1b2b3d40277029d4e609be2557d75221e3cfa
Dependencies: Stopgaps:

Status badges

Description (last modified by jdemeyer)

So we really need to know the exact matplotlib version number in sage-env?

# Use a matplotlib config directory specific to Sage and specific to
# the version number of matplotlib, by setting the environment
# variable MPLCONFIGDIR. Note that we can't find the version number by
# importing matplotlib, because that could create matplotlib's standard
# config directory. So we use pkg_resources.
"$SAGE_LOCAL/bin/python" -c 'import pkg_resources; pkg_resources.get_distribution("matplotlib").version' 2>/dev/null
if [ $? -eq 0 ]; then
    MPLVERSION=`"$SAGE_LOCAL/bin/python" -c 'import pkg_resources; print (pkg_resources.get_distribution("matplotlib").version)'`
    MPLCONFIGDIR="$DOT_SAGE/matplotlib-$MPLVERSION"
    export MPLCONFIGDIR
    # The directory is created when Sage starts (see sage.misc.misc).
fi

Instead, we could just hardcode a version number, analogous to what we do for Jupyter:

if [ -z "$IPYTHONDIR" ]; then
    # We hardcode a version number in the directory name. The idea is
    # that we keep using the same version number as long as that is
    # possible. Only when some future IPython version really requires
    # a new structure for the $IPYTHONDIR should this version number be
    # changed to the new IPython version.
    export IPYTHONDIR="$DOT_SAGE/ipython-5.0.0"
fi

if [ -z "$JUPYTER_CONFIG_DIR" ]; then
    # We hardcode a version number in the directory name. The idea is
    # that we keep using the same version number as long as that is
    # possible. Only when some future Jupyter version really requires
    # a new structure for the $JUPYTER_CONFIG_DIR should this version
    # number be changed to the new jupyter_core version.
    export JUPYTER_CONFIG_DIR="$DOT_SAGE/jupyter-4.1"
fi

Change History (33)

comment:1 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:2 Changed 4 years ago by jdemeyer

  • Branch set to u/jdemeyer/sage_env__do_not_depend_on_matplotlib_version_number

comment:3 Changed 4 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Commit set to 1f8c22ea34f2228f9fa977d79f51f19b3a85d5b1
  • Status changed from new to needs_review

New commits:

1f8c22eHardcode Matplotlib version number in sage-env

comment:4 follow-up: Changed 4 years ago by dimpase

Shouldn't this kind of hardcoding be delegated to ./configure ?

comment:5 Changed 4 years ago by embray

  • Authors Jeroen Demeyer deleted

That was fast. I'm sort of inclined to agree that the config dir really doesn't need to be updated unless an explicit need to do so is found.

An alternative approach I had in mind was to set the paths to these config files in a separate file that can be updated when upgrading the relevant packages. This would be a good use case for #22652, which I still think would be a good idea, though it hasn't been a priority to pursue. With #22652, individual spkgs could provide small shell snippets that are sourced at startup time. That keeps configuration for individual software packages closer to that package, rather than scattered about arbitrarily.


New commits:

1f8c22eHardcode Matplotlib version number in sage-env

comment:6 Changed 4 years ago by embray

  • Authors set to Jeroen Demeyer

Not sure why that happened.

comment:7 in reply to: ↑ 4 ; follow-up: Changed 4 years ago by jdemeyer

Replying to dimpase:

Shouldn't this kind of hardcoding be delegated to ./configure ?

No, how would ./configure know which version number to hardcode?

comment:8 Changed 4 years ago by fbissey

But #22652 is probably the right way to go on the long term. Not that I should really care.

comment:9 Changed 4 years ago by embray

  • Reviewers set to Erik Bray
  • Status changed from needs_review to positive_review

I think this makes sense to me, especially since we're already doing the same for Jupyter.

I'd still like a more general approach like I suggested above, but that can happen separately since this will give speedup times to sage startup now.

comment:10 follow-up: Changed 4 years ago by jdemeyer

I'm not convinced to use #22652 to change the directory every time. Usually, it is considered a feature that a configuration directory remains in the same place.

That was the reason for hardcoding the Jupyter directories: I have some custom configuration and it's not fun if you need to copy it every time that Jupyter is upgraded in Sage.

comment:11 in reply to: ↑ 7 ; follow-up: Changed 4 years ago by dimpase

Replying to jdemeyer:

Replying to dimpase:

Shouldn't this kind of hardcoding be delegated to ./configure ?

No, how would ./configure know which version number to hardcode?

trivially: by looking at build/pkgs/*/package-version.txt

comment:12 follow-up: Changed 4 years ago by fbissey

If I remember well, the version hardcoding is meant to change when incompatible changes are made. #22652 just give the package the responsibility to increment the number, it can mimic the current behavior and only change the number at breaking point.

comment:13 Changed 4 years ago by dimpase

  • Status changed from positive_review to needs_work

by right, these kinds of changes must be documented somewhere (in the developer manual, or/and in SPKG.txt), otherwise you're building a code spaghetti monster only you know the way around...

comment:14 in reply to: ↑ 11 ; follow-up: Changed 4 years ago by jdemeyer

Replying to dimpase:

trivially: by looking at build/pkgs/*/package-version.txt

That's exactly what we do not want to do. It's a misfeature to have configuration directories depend on a version number.

comment:15 in reply to: ↑ 10 Changed 4 years ago by embray

Replying to jdemeyer:

I'm not convinced to use #22652 to change the directory every time. Usually, it is considered a feature that a configuration directory remains in the same place.

I'm not necessarily saying it should be changed every time. It could be done that way, and more easily if the config is written, say, by the package's spkg-install. My point here is that it just keeps this configuration closer to the package it's relevant to, making it easier to remember to update its configuration when desired/necessary.

comment:16 in reply to: ↑ 12 Changed 4 years ago by embray

Replying to fbissey:

If I remember well, the version hardcoding is meant to change when incompatible changes are made. #22652 just give the package the responsibility to increment the number, it can mimic the current behavior and only change the number at breaking point.

Exactly

comment:17 in reply to: ↑ 14 ; follow-up: Changed 4 years ago by dimpase

Replying to jdemeyer:

Replying to dimpase:

trivially: by looking at build/pkgs/*/package-version.txt

That's exactly what we do not want to do. It's a misfeature to have configuration directories depend on a version number.

Tell me more about advantages of having the same number hardcoded in two different files, come on...

You are perpetuating some old crappy misdesign, not the other way around.

Last edited 4 years ago by dimpase (previous) (diff)

comment:18 in reply to: ↑ 17 ; follow-up: Changed 4 years ago by embray

Replying to dimpase:

Replying to jdemeyer:

Replying to dimpase:

trivially: by looking at build/pkgs/*/package-version.txt

That's exactly what we do not want to do. It's a misfeature to have configuration directories depend on a version number.

Tell me more about advantages of having the same number hardcoded in two different files, come on...

You are perpetuating some old crappy misdesign, not the other way around.

Jeroen's point is that it's not the same number. The number in the config directory name is the last version at which the format of the configuration for that package changed in a significantly-enough backwards-incompatible way that a new configuration is needed.

For example "jupyter-1.5.0" means "configuration for jupyter-1.5.0 and above". If jupyter 3.0 comes along and changes the layout of its config directory then we add "jupyter-3.0" for config of jupyter 3.0 and above.

This is as opposed to updating the config dir every time a package is updated (especially onerous when we're talking micro version updates that only contain bug fixes). The config dirs do not track one-to-one with the version of the package.

comment:19 in reply to: ↑ 18 ; follow-up: Changed 4 years ago by dimpase

Replying to embray:

Replying to dimpase:

Replying to jdemeyer:

Replying to dimpase:

trivially: by looking at build/pkgs/*/package-version.txt

That's exactly what we do not want to do. It's a misfeature to have configuration directories depend on a version number.

Tell me more about advantages of having the same number hardcoded in two different files, come on...

You are perpetuating some old crappy misdesign, not the other way around.

Jeroen's point is that it's not the same number. The number in the config directory name is the last version at which the format of the configuration for that package changed in a significantly-enough backwards-incompatible way that a new configuration is needed.

For example "jupyter-1.5.0" means "configuration for jupyter-1.5.0 and above". If jupyter 3.0 comes along and changes the layout of its config directory then we add "jupyter-3.0" for config of jupyter 3.0 and above.

This is as opposed to updating the config dir every time a package is updated (especially onerous when we're talking micro version updates that only contain bug fixes). The config dirs do not track one-to-one with the version of the package.

This is called madness in my book. I would understand these directory names being independent of the minor version, but having in ~/.sage/blah-1.1.1 configuration for blah-2.3.4 is crazy.

comment:20 in reply to: ↑ 19 Changed 4 years ago by jdemeyer

Replying to dimpase:

having in ~/.sage/blah-1.1.1 configuration for blah-2.3.4 is crazy.

You should consider the 1.1.1 to be essentially an arbitrary string. One could go with ~/.sage/blah-A if you want. The reason that I picked 1.5.1 is because that is what is currently used, so the directory wouldn't be changed.

comment:21 Changed 4 years ago by embray

Let's put it this way: At the very least this ticket is just treating matplotlib's config the way we were already treating jupyter/ipython config, so if nothing else it's consistent. I think it makes sense to name the config dir after the lowest applicable version supported by Sage, but you could just as well name them ".0", ".1", ".2", etc. This would be a debate for a separate ticket.

comment:22 Changed 4 years ago by git

  • Commit changed from 1f8c22ea34f2228f9fa977d79f51f19b3a85d5b1 to 5b9481cc061ee03484020b6025b8321181793311

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

5b9481cDocument MPLCONFIGDIR

comment:23 Changed 4 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:24 Changed 4 years ago by dimpase

OK, thanks, this starts making sense to me. Now it remains to check that the docs build...

comment:25 Changed 4 years ago by chapoton

typo "where the configuation"

comment:26 Changed 4 years ago by git

  • Commit changed from 5b9481cc061ee03484020b6025b8321181793311 to e45a772568b398e8415a839a7248d2c33f2deb25

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

e45a772Typos

comment:27 Changed 4 years ago by jdemeyer

ping

comment:28 Changed 4 years ago by embray

I was waiting to see if Dima had anything more to say about this--it has positive review from me otherwise.

comment:29 Changed 4 years ago by dimpase

just a second, I'll see if it works on top of #23023

comment:30 Changed 4 years ago by dimpase

OK, but how about a link to https://matplotlib.org/faq/environment_variables_faq.html#envvar-MPLCONFIGDIR in the place where you mention MPLCONFIGDIR in the docs?

comment:31 Changed 4 years ago by git

  • Commit changed from e45a772568b398e8415a839a7248d2c33f2deb25 to e3c1b2b3d40277029d4e609be2557d75221e3cfa

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

e3c1b2bAdd link

comment:32 Changed 4 years ago by dimpase

  • Reviewers changed from Erik Bray to Erik Bray, Dima Pasechnik
  • Status changed from needs_review to positive_review

OK, thanks.

comment:33 Changed 4 years ago by vbraun

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