Opened 6 months ago

Closed 6 months ago

#15972 closed defect (fixed)

IPython ProfileDirError if IPython was never run

Reported by: jdemeyer Owned by:
Priority: blocker Milestone: sage-6.2
Component: interfaces Keywords:
Cc: vbraun, jason, was, roed Merged in:
Authors: John Palmieri Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: 53a7d7f (Commits) Commit: 53a7d7f6e6bcac537e5442be47c50b197bc571f1
Dependencies: Stopgaps:

Description (last modified by 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

Change History (16)

comment:1 Changed 6 months ago by jhpalmieri

This seems to fix it. Is the right approach?

  • src/sage/misc/interpreter.py

    diff --git a/src/sage/misc/interpreter.py b/src/sage/misc/interpreter.py
    index aff78f7..349e9dc 100644
    a b class SageTerminalApp(TerminalIPythonApp): 
    614614 
    615615    def load_config_file(self, *args, **kwds): 
    616616        from IPython.config.loader import PyFileConfigLoader, ConfigFileNotFound 
    617         from IPython.core.profiledir import ProfileDir 
     617        from IPython.core.profiledir import ProfileDir, ProfileDirError 
    618618        from IPython.utils.path import get_ipython_dir 
    619619 
    620620        conf = Config() 
    class SageTerminalApp(TerminalIPythonApp): 
    622622        conf._merge(self.command_line_config) 
    623623 
    624624        # Get user config. 
    625         sage_profile_dir = ProfileDir.find_profile_dir_by_name( 
    626             get_ipython_dir(), 'sage').location 
     625        try: 
     626            sage_profile_dir = ProfileDir.find_profile_dir_by_name( 
     627                get_ipython_dir(), 'sage').location 
     628        except ProfileDirError: 
     629            d = ProfileDir.create_profile_dir_by_name( 
     630                get_ipython_dir(), 'sage') 
     631            sage_profile_dir = d.location 
    627632        try: 
    628633            cl = PyFileConfigLoader('ipython_config.py', sage_profile_dir) 
    629634            conf._merge(cl.load_config()) 

comment:2 Changed 6 months ago by vbraun

looks good to me

comment:3 Changed 6 months ago by jhpalmieri

  • Authors set to John Palmieri
  • Branch set to u/jhpalmieri/ipython-profile-dir
  • Commit set to 7889b1aa5cb7f113a61e0d7f06e3a353637ca95c
  • Status changed from new to needs_review

Okay, here's a branch with the changes.


New commits:

7889b1aCreate IPython config dir if necessary

comment:4 Changed 6 months ago by ohanar

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

comment:5 Changed 6 months ago by jdemeyer

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?

comment:6 Changed 6 months ago by ohanar

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

comment:7 Changed 6 months ago by vbraun

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.

comment:8 Changed 6 months ago by jhpalmieri

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.

comment:9 follow-up: Changed 6 months ago by vbraun

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.

comment:10 Changed 6 months ago by jhpalmieri

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.

comment:11 Changed 6 months ago by jhpalmieri

How about this?

comment:12 Changed 6 months ago by git

  • Commit changed from 7889b1aa5cb7f113a61e0d7f06e3a353637ca95c to 53a7d7f6e6bcac537e5442be47c50b197bc571f1

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

53a7d7fadd doctests for #15972

comment:13 Changed 6 months ago by jdemeyer

  • Description modified (diff)

comment:14 in reply to: ↑ 9 Changed 6 months ago by jdemeyer

  • Description modified (diff)

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).

comment:15 Changed 6 months ago by vbraun

  • Reviewers set to Volker Braun

comment:16 Changed 6 months ago by vbraun

  • Branch changed from u/jhpalmieri/ipython-profile-dir to 53a7d7f6e6bcac537e5442be47c50b197bc571f1
  • Resolution set to fixed
  • Status changed from needs_review to closed
Note: See TracTickets for help on using tickets.