IPython ProfileDirError if IPython was never run

[jdemeyer]

The following fails if IPython was never run before:

./sage -t src/sage/misc/ascii_art.py
**********************************************************************
File "src/sage/misc/ascii_art.py", line 42, in sage.misc.ascii_art
Failed example:
    shell = get_test_shell()
Exception raised:
    Traceback (most recent call last):
      File "/usr/local/src/sage-config/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 480, in _run
        self.execute(example, compiled, test.globs)
      File "/usr/local/src/sage-config/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 839, in execute
        exec compiled in globs
      File "<doctest sage.misc.ascii_art[6]>", line 1, in <module>
        shell = get_test_shell()
      File "/usr/local/src/sage-config/local/lib/python2.7/site-packages/sage/misc/interpreter.py", line 549, in get_test_shell
        app.initialize(argv=[])
      File "<string>", line 2, in initialize
      File "/usr/local/src/sage-config/local/lib/python2.7/site-packages/IPython/config/application.py", line 89, in catch_config_error
        return method(app, *args, **kwargs)
      File "/usr/local/src/sage-config/local/lib/python2.7/site-packages/IPython/terminal/ipapp.py", line 312, in initialize
        super(TerminalIPythonApp, self).initialize(argv)
      File "<string>", line 2, in initialize
      File "/usr/local/src/sage-config/local/lib/python2.7/site-packages/IPython/config/application.py", line 89, in catch_config_error
        return method(app, *args, **kwargs)
      File "/usr/local/src/sage-config/local/lib/python2.7/site-packages/IPython/core/application.py", line 381, in initialize
        self.load_config_file()
      File "/usr/local/src/sage-config/local/lib/python2.7/site-packages/sage/misc/interpreter.py", line 626, in load_config_file
        get_ipython_dir(), 'sage').location
      File "/usr/local/src/sage-config/local/lib/python2.7/site-packages/IPython/core/profiledir.py", line 242, in find_profile_dir_by_name
        raise ProfileDirError('Profile directory not found in paths: %s' % dirname)
    ProfileDirError: Profile directory not found in paths: profile_sage
**********************************************************************

Running ./sage once fixes this problem but the following always fails:

./sage --nodotsage -t src/sage/misc/ascii_art.py

CC: @vbraun @jasongrout @williamstein @roed314

Component: interfaces

Author: John Palmieri

Branch/Commit: 53a7d7f

Reviewer: Volker Braun

Issue created by migration from https://trac.sagemath.org/ticket/15972

[jhpalmieri]

comment:1

This seems to fix it. Is the right approach?

diff --git a/src/sage/misc/interpreter.py b/src/sage/misc/interpreter.py
index aff78f7..349e9dc 100644
--- a/src/sage/misc/interpreter.py
+++ b/src/sage/misc/interpreter.py
@@ -614,7 +614,7 @@ class SageTerminalApp(TerminalIPythonApp):
 
     def load_config_file(self, *args, **kwds):
         from IPython.config.loader import PyFileConfigLoader, ConfigFileNotFound
-        from IPython.core.profiledir import ProfileDir
+        from IPython.core.profiledir import ProfileDir, ProfileDirError
         from IPython.utils.path import get_ipython_dir
 
         conf = Config()
@@ -622,8 +622,13 @@ class SageTerminalApp(TerminalIPythonApp):
         conf._merge(self.command_line_config)
 
         # Get user config.
-        sage_profile_dir = ProfileDir.find_profile_dir_by_name(
-            get_ipython_dir(), 'sage').location
+        try:
+            sage_profile_dir = ProfileDir.find_profile_dir_by_name(
+                get_ipython_dir(), 'sage').location
+        except ProfileDirError:
+            d = ProfileDir.create_profile_dir_by_name(
+                get_ipython_dir(), 'sage')
+            sage_profile_dir = d.location
         try:
             cl = PyFileConfigLoader('ipython_config.py', sage_profile_dir)
             conf._merge(cl.load_config())

[vbraun]

comment:2

looks good to me

[jhpalmieri]

comment:3

Okay, here's a branch with the changes.

---

New commits:

7889b1a

Create IPython config dir if necessary

[jhpalmieri]

Author: John Palmieri

[jhpalmieri]

Commit: 7889b1a

[jhpalmieri]

Branch: u/jhpalmieri/ipython-profile-dir

[ohanar]

comment:4

Is there any reasonable way to add tests for this method?

[jdemeyer]

comment:5

Testing with --nodotsage is something which I used to do when testing releases (together with various other unusual setups, such as running Sage as a different user from the one which owns the Sage install). Volker, could you do this on (some of the) buildbots?

[ohanar]

comment:6

I more meant are there any reasonable doctests that could be added to this method? (Although what you mentioned is good as well)

[vbraun]

comment:7

I don't understand the description. Does running ./sage once fix the command invocation or not?

Tests that combinations of commandline parameters work belong into doctests, not in a special buildbot config.

[jhpalmieri]

comment:8

If a user has never run Sage before, they will see this error when doing ./sage -t src/sage/misc/ascii_art.py. Once they've run Sage once, it creates their IPython config directory, so the error doesn't occur any more (when running vanilla ./sage -t FILE). If they run ./sage --nodotsage -t src/sage/misc/ascii_art.py, though, then the error reappears, because there is no existing IPython config directory when you run with --nodotsage.

What Jeroen is saying is that all doctests should pass when you run with --nodotsage (or with a different user, etc.); it's not specific to this particular error. I suppose we could put a test like ./sage --nodotsage -t SOME/FILE in tests/cmdline.py, but that seems a little odd to me.

[vbraun]

comment:9

So you are saying that it fails without --nodotsage before starting sage the first time? This is not what the description says.

You don't test the ipython profile creation any better by running the entire testsuite with --nodotsage. Command line switches must be tested via doctests, anything after the review is too late. The release manager role is not a second reviewer for every ticket, that does not scale.

[jhpalmieri]

comment:10

I didn't write the description, but I think that --nodoctest could safely be removed from the second line. Delete the directory .sage/ipython-1.2.1/ and try the doctest. It fails for me.

As far as reviewing goes, this should have been caught at #14713 (by the reviewer running tests with --nodotsage, for example -- it should have been caught before it got to the release manager). Or both you and I should have done a better job at #14188 when the load_config_file method was introduced (without any doctests). Anyway, I'm open to any suggestions about how to doctest this.

[jhpalmieri]

comment:11

How about this?

[sagetrac-git]

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

53a7d7f

add doctests for #15972

[sagetrac-git]

Changed commit from 7889b1a to 53a7d7f

[jdemeyer]

comment:14

Replying to @vbraun:

So you are saying that it fails without --nodotsage before starting sage the first time? This is not what the description says.

Fixed that.

Command line switches must be tested via doctests

Testing everything with --nodotsage doubles the doctest time, testing everything as a user which doesn't have write access to SAGE_ROOT isn't even possible in doctests.

The release manager role is not a second reviewer for every ticket, that does not scale.

In which sense does it "not scale"? I did that and it worked for me perfectly, there are very few tickets which would fail this test. The release manager already is a second reviewer, given that something like 10% of all positively reviewed tickets simply fail when tested on the buildbots (maybe less now that docbuild errors are caught earlier).

[vbraun]

Reviewer: Volker Braun

[vbraun]

Changed branch from u/jhpalmieri/ipython-profile-dir to 53a7d7f