Opened 3 years ago

Closed 3 years ago

#21430 closed defect (fixed)

Set JUPYTER_CONFIG_DIR

Reported by: jdemeyer Owned by:
Priority: blocker Milestone: sage-7.4
Component: scripts Keywords:
Cc: chapoton Merged in:
Authors: Jeroen Demeyer Reviewers: François Bissey, Erik Bray, Leif Leonhardy
Report Upstream: N/A Work issues:
Branch: 1beaae3 (Commits) Commit: 1beaae37abc0d484688fa08363207770e59b0a43
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

We should set JUPYTER_CONFIG_DIR to some directory inside $DOT_SAGE, similar to what we do with other configuration directories. Note that this directory is not created when just running Sage, only when doing something involving Jupyter.

We also hardcode the version number of IPYTHONDIR, which was always the intention, see https://trac.sagemath.org/ticket/12719#comment:225 This way, users who want to customize their IPython configuration do not need to redo that whenever we upgrade IPython.

Change History (47)

comment:1 Changed 3 years ago by jdemeyer

  • Cc chapoton added

comment:2 Changed 3 years ago by vbraun

I would still try to version the directory name as this caused breakage before

comment:3 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:4 Changed 3 years ago by jdemeyer

  • Branch set to u/jdemeyer/set_jupyter_config_dir

comment:5 Changed 3 years ago by jdemeyer

  • Commit set to a3b040f026d3522d8f07e0339d86370e0824bcc4
  • Description modified (diff)
  • Status changed from new to needs_review

New commits:

a3b040fSet JUPYTER_CONFIG_DIR to ~/.sage/jupyter and hardcode IPYTHONDIR

comment:6 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:7 follow-ups: Changed 3 years ago by embray

Versioning the directory name is a good idea, but I'm not sure about hard-coding it sage-env. There is no obvious pointer that this would need to be updated when updating the IPython/Jupyter versions in Sage. This will inevitably bit-rot. Instead it should read the package version from somewhere else.

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

Replying to embray:

Versioning the directory name is a good idea, but I'm not sure about hard-coding it sage-env. There is no obvious pointer that this would need to be updated when updating the IPython/Jupyter versions in Sage. This will inevitably bit-rot. Instead it should read the package version from somewhere else.

You are misunderstanding. The hardcoding is intentional, the idea is not to update it every time that IPython/Jupyter gets updated. I thought that the comments in sage-env explain this, but apparently that is not clear.

comment:9 in reply to: ↑ 7 ; follow-up: Changed 3 years ago by leif

Replying to embray:

Versioning the directory name is a good idea, but I'm not sure about hard-coding it sage-env. There is no obvious pointer that this would need to be updated when updating the IPython/Jupyter versions in Sage. This will inevitably bit-rot. Instead it should read the package version from somewhere else.

Hmmm, the spkg-install files currently don't seem very suited for a sanity-check, although they could parse their own package-version.txt and compare that to the env var(s). I don't trust upstream versioning though.

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

Replying to jdemeyer:

Replying to embray:

Versioning the directory name is a good idea, but I'm not sure about hard-coding it sage-env. There is no obvious pointer that this would need to be updated when updating the IPython/Jupyter versions in Sage. This will inevitably bit-rot. Instead it should read the package version from somewhere else.

You are misunderstanding. The hardcoding is intentional, the idea is not to update it every time that IPython/Jupyter gets updated. I thought that the comments in sage-env explain this, but apparently that is not clear.

I think he rather meant who will remember to change sage-env when one upgrades to IPython 6.x (presumably next week ;-) ).

comment:11 Changed 3 years ago by embray

No, the comment was pretty vague. I don't know what "IPython versions compatible with that version" means.

comment:12 in reply to: ↑ 9 ; follow-ups: Changed 3 years ago by embray

Replying to leif:

Replying to embray:

Versioning the directory name is a good idea, but I'm not sure about hard-coding it sage-env. There is no obvious pointer that this would need to be updated when updating the IPython/Jupyter versions in Sage. This will inevitably bit-rot. Instead it should read the package version from somewhere else.

Hmmm, the spkg-install files currently don't seem very suited for a sanity-check, although they could parse their own package-version.txt and compare that to the env var(s). I don't trust upstream versioning though.

I'm thinking each spkg should have its own "sage environment setup participant hook"--i.e. "here is a bash snippet specific to this package that should be sourced during sage-env." That would go a long way toward making sage-env more comprehensible, and keeping spkgs better self-contained.

comment:13 Changed 3 years ago by git

  • Commit changed from a3b040f026d3522d8f07e0339d86370e0824bcc4 to 78f964e494bd54b57b3dc456e536046ff4485e17

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

78f964eClarify comment (hopefully)

comment:14 in reply to: ↑ 12 Changed 3 years ago by leif

Replying to embray:

I'm thinking each spkg should have its own "sage environment setup participant hook"--i.e. "here is a bash snippet specific to this package that should be sourced during sage-env." That would go a long way toward making sage-env more comprehensible, and keeping spkgs better self-contained.

Yes, tag regions of the shell script with # optional -- foo, bar...

comment:15 Changed 3 years ago by leif

I guess you'll quickly end up in a dependency hell.

comment:16 in reply to: ↑ 12 Changed 3 years ago by jdemeyer

Replying to embray:

I'm thinking each spkg should have its own "sage environment setup participant hook"--i.e. "here is a bash snippet specific to this package that should be sourced during sage-env." That would go a long way toward making sage-env more comprehensible, and keeping spkgs better self-contained.

Perhaps, but that's besides the point of this ticket.

comment:17 follow-up: Changed 3 years ago by fbissey

I guess Erik is thinking of something akin to /etc/env.d or /etc/profile.d but sourced from build/pkg/$pkgname/env. An interesting idea in principle. it doesn't have to be involved with dependencies.

But I was OK with the old comments and I am OK with the new one.

comment:18 in reply to: ↑ 17 Changed 3 years ago by leif

Replying to fbissey:

I guess Erik is thinking of something akin to /etc/env.d or /etc/profile.d but sourced from build/pkg/$pkgname/env. An interesting idea in principle. it doesn't have to be involved with dependencies.

But I was OK with the old comments and I am OK with the new one.

Still, this needs to be documented in build/pkgs/*. (Who reads SPKG.txt? Better also in spkg-install IMHO, if not even adding a sanity check as mentioned.)

comment:19 follow-up: Changed 3 years ago by leif

Doesn't seem we yet do similar for ipython_genutils.

comment:20 in reply to: ↑ 19 Changed 3 years ago by leif

Replying to leif:

Doesn't seem we yet do similar for ipython_genutils.

Ooops, but that's yet another build/pkgs/foo/....

comment:21 follow-up: Changed 3 years ago by fbissey

  • Reviewers set to François Bissey, Erik Bray
  • Status changed from needs_review to positive_review

Enough bike shedding. Positive review.

comment:22 in reply to: ↑ 21 ; follow-up: Changed 3 years ago by leif

  • Reviewers changed from François Bissey, Erik Bray to François Bissey, Erik Bray, Leif Leonhardy
  • Status changed from positive_review to needs_work

Replying to fbissey:

Enough bike shedding. Positive review.

I'll remind you next time you want something documented.

(These environment variables aren't even documented in the Sage Installation Guide, where all environment variables used by Sage, not necessarily supposed to be set or modified by the user, are documented for historical reasons.)


But definitely needs work because you're not exactly hardcoding the folders; if a user happens to have them already set in his/her environment, this is asking for trouble.

comment:23 in reply to: ↑ 22 ; follow-up: Changed 3 years ago by jdemeyer

  • Status changed from needs_work to needs_review

Replying to leif:

(These environment variables aren't even documented in the Sage Installation Guide, where all environment variables used by Sage, not necessarily supposed to be set or modified by the user, are documented for historical reasons.)

I don't think it makes sense for Sage to document all environment variables used by all packages of Sage. It should document Sage-specific environment variables, not those of packages inside the Sage distribution.

But definitely needs work because you're not exactly hardcoding the folders; if a user happens to have them already set in his/her environment, this is asking for trouble.

No, if the user has them already set in his environment, we assume that the user knows what he is doing. I see the JUPYTER_CONFIG_DIR in sage-env simply as a reasonable default that users should still be able to override.

comment:24 in reply to: ↑ 23 ; follow-up: Changed 3 years ago by leif

Replying to jdemeyer:

Replying to leif:

(These environment variables aren't even documented in the Sage Installation Guide, where all environment variables used by Sage, not necessarily supposed to be set or modified by the user, are documented for historical reasons.)

I don't think it makes sense for Sage to document all environment variables used by all packages of Sage. It should document Sage-specific environment variables, not those of packages inside the Sage distribution.

We don't document variables that are solely set and used internally, but those the user may want to set or modify, those that Sage sets and a user/developer may find to be valuable to use, and those that we change although the user is likely to expect we honor his/her setting, for example MPLCONFIGDIR.


But definitely needs work because you're not exactly hardcoding the folders; if a user happens to have them already set in his/her environment, this is asking for trouble.

No, if the user has them already set in his environment, we assume that the user knows what he is doing.

Besides that your assumption is presumably wrong in most cases, then "hardcoding" is the wrong term.


I see the JUPYTER_CONFIG_DIR in sage-env simply as a reasonable default that users should still be able to override.

IPYTHONDIR as well? PYTHONPATH? PYTHONHOME? What else?

(Btw., CC, CXX etc. get overridden in case Sage's GCC is installed; why don't you respect a user's setting there as well?)

If you really want to create more of such pitfalls, the need to document this is even greater, i.e., it needs to get mentioned in more prominent locations such as READMEs as well, and a warning should get issued unless it is totally clear the user did intentionally override one of these "default" settings. You'll get error reports because of this anyway, but hopefully less. But it would still be better to provide other, more explicit ways to override Sage's "default" settings if you really want to support that. Sage's configure for example also bails out in a couple of cases unless one sets SAGE_PORT=yes, which is a pretty similar situation, although this affects even less people, namely only those building from source.

Only a tiny percentage of the Sage users will even know these environment variables exist, what their meaning is, and whether they're perhaps (unintentionally) set in their environment. Just read a bit in sage-support, or on ask.sagemath.org. Even "Sage developers" are mostly mathematicians, and not all of them have computers, operating systems, shells etc. as their beloved hobby.

comment:25 Changed 3 years ago by leif

P.S.: I'm absolutely no fan of nannying people, but we should follow the principle of least surprise (and disappointment) also for those not that deep in Sage's (and its packages') matter.

After all, there are configuration files those that want to can modify. Still no .sagerc though, if I'm not mistaken.

comment:26 in reply to: ↑ 24 ; follow-up: Changed 3 years ago by jdemeyer

It is still not clear to me why Sage should document environment variables which have nothing to do with Sage but which are for other projects.

comment:27 in reply to: ↑ 26 ; follow-up: Changed 3 years ago by leif

Replying to jdemeyer:

It is still not clear to me why Sage should document environment variables which have nothing to do with Sage but which are for other projects.

Such as?

comment:28 in reply to: ↑ 27 Changed 3 years ago by jdemeyer

Replying to leif:

Replying to jdemeyer:

It is still not clear to me why Sage should document environment variables which have nothing to do with Sage but which are for other projects.

Such as?

JUPYTER_CONFIG_DIR what this ticket is about. It is perfectly fine if users don't know about this variable.

comment:29 follow-up: Changed 3 years ago by leif

But you're doing the same with IPYTHONDIR (although not mentioned in the ticket's title, and the description is misleading as well).

More precisely, you're introducing sharing folders (hence configuration) between unrelated and potentially different/incompatible instances of IPython. This is essentially the same as with Python and PYTHONUSERBASE. It might work to some extent (if you're lucky), but is more likely to cause trouble unless you really know/are aware of what you (and probably others) are doing, so IMHO nothing for "ordinary" users. (Not to mention distros will mess with "their" installations as they like, and a user may not even know or immediately notice.)

Using the same folder for compatible versions of Sage's IPython makes sense, but nothing prevents people from (accidentally) using a folder outside of $DOT_SAGE, and especially not from using the system's IPython's folder, and no matter what version the system's IPython is, as the variable name itself does not carry a version, because it is assumed there is exactly one "active" version.

comment:30 in reply to: ↑ 29 ; follow-up: Changed 3 years ago by jdemeyer

Replying to leif:

nothing prevents people from (accidentally) using a folder outside of $DOT_SAGE

which is a problem because .......... (fill in the blank)

comment:31 in reply to: ↑ 30 ; follow-up: Changed 3 years ago by leif

Replying to jdemeyer:

Replying to leif:

nothing prevents people from (accidentally) using a folder outside of $DOT_SAGE

which is a problem because .......... (fill in the blank)

... that already violates the general convention of Sage to not modify anything outside $SAGE_ROOT, $DOT_SAGE (by default $HOME/.sage/), besides $TMPDIR.

Other exceptions are for example $SAGE_BUILD_DIR, but

  • that's well-documented,
  • it defaults to some location below $SAGE_ROOT,
  • the user has to explicitly set it,
  • there can't be any confusion (or unintentional collision) because the variable name starts with SAGE_, so (hopefully) isn't used by any other application,
  • we don't store persistent things, especially no configuration, there,
  • ...

But you of course missed the more important point.

comment:32 in reply to: ↑ 31 ; follow-up: Changed 3 years ago by jdemeyer

Replying to leif:

... that already violates the general convention of Sage to not modify anything outside $SAGE_ROOT, $DOT_SAGE (by default $HOME/.sage/), besides $TMPDIR.

I agree that this should be the case by default. However, if the user explicitly asks for a different directory, we should honor that.

But you of course missed the more important point.

Well, you are writing so much stuff in the comments here, most of which is unrelated to the topic of this ticket. So it's easy to miss the important point. Honestly, I cannot really figure out which point you are making.

comment:33 in reply to: ↑ 32 ; follow-up: Changed 3 years ago by leif

Replying to jdemeyer:

Replying to leif:

... that already violates the general convention of Sage to not modify anything outside $SAGE_ROOT, $DOT_SAGE (by default $HOME/.sage/), besides $TMPDIR.

I agree that this should be the case by default. However, if the user explicitly asks for a different directory, we should honor that.

But you of course missed the more important point.

Well, you are writing so much stuff in the comments here, most of which is unrelated to the topic of this ticket. So it's easy to miss the important point. Honestly, I cannot really figure out which point you are making.

Well, you're just not willing to get the point, instead quoting just subsentences and asking for details regarding less important things. Which part of "accidentally", "unintentionally" or "not set by the user itself" did you not understand?

The bare fact that IPYTHONDIR is (already) set means nothing, and especially not that it was set for the (same) version of IPython Sage ships and uses.

(And you didn't answer either why in your opinion in contrast SAGE_PORT=yes is necessary, CC needs to get overridden, why you force some compiler flags regardless of the context, or why MAKE="$MAKE -j1" should be necessary just because you personally once did get a failure without it. That's just inconsistent to what you propose here. In addition, IPYTHONDIR affects any Sage user, not just developers or people building from source.)

comment:34 in reply to: ↑ 33 ; follow-up: Changed 3 years ago by jdemeyer

Replying to leif:

instead quoting just subsentences and asking for details regarding less important things.

Again, the more details you write, the easier it is for me to get lost in those details.

Which part of "accidentally", "unintentionally" or "not set by the user itself" did you not understand?

I just don't understand how environment variables can be set accidentally.

CC needs to get overridden

Well, we only override CC if the GCC package was installed. That makes sense because it means that the system CC was not good enough.

why MAKE="$MAKE -j1" should be necessary

Which MAKE="$MAKE -j1" do you mean?

comment:35 in reply to: ↑ 34 Changed 3 years ago by embray

Replying to jdemeyer:

Replying to leif:

Which part of "accidentally", "unintentionally" or "not set by the user itself" did you not understand?

I just don't understand how environment variables can be set accidentally.

Try teaching a workshop sometime. You would not believe the stuff users have in their .profile with literally no idea how it got there (usually something they added 13 months ago while feverishly copy-pasting and otherwise punching at the darkness while trying to get some .c file their professor e-mailed them to compile :)

comment:36 follow-up: Changed 3 years ago by embray

Also, I haven't read this entire thread but just my $0.02 USD--I think it's appropriate to document environment variables that pertain to Sage--either that affect how Sage runs, or that Sage itself affects--even if they are not specific to Sage. Yes there's a slippery slope, but some variables may have more unexpected impact than others and are less generally well-understood.

comment:37 in reply to: ↑ 36 ; follow-ups: Changed 3 years ago by jdemeyer

Replying to embray:

Yes there's a slippery slope, but some variables may have more unexpected impact than others and are less generally well-understood.

I still think it's not the job of Sage to document all Jupyter-related environment variables. Maybe a compromise could be to add a pointer to the Jupyter documentation: http://jupyter.readthedocs.io/en/latest/projects/jupyter-directories.html

comment:38 in reply to: ↑ 37 Changed 3 years ago by fbissey

Replying to jdemeyer:

Replying to embray:

Yes there's a slippery slope, but some variables may have more unexpected impact than others and are less generally well-understood.

I still think it's not the job of Sage to document all Jupyter-related environment variables. Maybe a compromise could be to add a pointer to the Jupyter documentation: http://jupyter.readthedocs.io/en/latest/projects/jupyter-directories.html

While it may not be the job to document other package variables, there is a case for documentation when sage change their default values. An experienced jupyter user may be wondering why they don't find stuff in ~/.jupyter it should be easy for them to find out about sage's setting (other than reading sage-env).

comment:39 follow-up: Changed 3 years ago by klee

How about adding "+" at the end: export IPYTHONDIR="$DOT_SAGE/ipython-5.0.0+"?

comment:40 in reply to: ↑ 39 Changed 3 years ago by jdemeyer

Replying to klee:

How about adding "+" at the end: export IPYTHONDIR="$DOT_SAGE/ipython-5.0.0+"?

What does this + symbol mean?

comment:41 follow-up: Changed 3 years ago by klee

To mean that the configuration applies not only to ipython-5.0.0 but also to all higher versions (until upgraded to a non-compatible version)

comment:42 in reply to: ↑ 41 Changed 3 years ago by jdemeyer

Replying to klee:

To mean that the configuration applies not only to ipython-5.0.0 but also to all higher versions (until upgraded to a non-compatible version)

Then you might as well call it ipython-a or whatever... I only chose 5.0.0 because that is the current version.

comment:43 in reply to: ↑ 37 Changed 3 years ago by embray

Replying to jdemeyer:

Replying to embray:

Yes there's a slippery slope, but some variables may have more unexpected impact than others and are less generally well-understood.

I still think it's not the job of Sage to document all Jupyter-related environment variables. Maybe a compromise could be to add a pointer to the Jupyter documentation: http://jupyter.readthedocs.io/en/latest/projects/jupyter-directories.html

Nobody said it's the job of Sage to document "all" Jupyter-related variables. Just ones that are commonly impacted by how Sage uses them.

comment:44 Changed 3 years ago by git

  • Commit changed from 78f964e494bd54b57b3dc456e536046ff4485e17 to 1beaae37abc0d484688fa08363207770e59b0a43

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

1beaae3Document IPYTHONDIR and JUPYTER_CONFIG_DIR

comment:45 Changed 3 years ago by jdemeyer

Still needs review...

comment:46 Changed 3 years ago by embray

  • Status changed from needs_review to positive_review

Thanks for adding the docs!

comment:47 Changed 3 years ago by vbraun

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