Opened 9 years ago
Closed 9 years ago
#12911 closed defect (fixed)
fix failing ipython test in tests/cmdline
Reported by: | jhpalmieri | Owned by: | jason |
---|---|---|---|
Priority: | major | Milestone: | sage-5.1 |
Component: | misc | Keywords: | ipython |
Cc: | Merged in: | sage-5.1.beta6 | |
Authors: | John Palmieri | Reviewers: | Keshav Kini |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
The test
sage: (out, err, ret) = test_executable(["sage", "--ipython"], "\n3**33\n") sage: out.find("5559060566555523") >= 0 True
fails if people have installed a newer version (0.12?) of ipython which leaves incompatible files in ~/ipython/
. We might consider setting IPYTHONDIR
to $DOT_SAGE/ipython/
in sage-env
, but the simplest fix seems to be to do this right before running the test.
Test this by running sage --ipython
to create config files in ~/ipython/
. Then edit ~/ipython/ipy_user_conf.py
to (for example) make it invalid Python, so it prints an error when you start sage --ipython
. Then doctest the file cmdline.py
before and after applying the patch.
Attachments (1)
Change History (12)
Changed 9 years ago by
comment:1 Changed 9 years ago by
- Description modified (diff)
- Status changed from new to needs_review
comment:2 Changed 9 years ago by
comment:3 Changed 9 years ago by
Oh, wait; rereading your description makes it seem like the problem is if they have a system 0.12, but Sage's 0.10, that causes a problem.
So installing #12719 won't solve the problem by itself, since modifying the global config file to be invalid python still would cause problems. However, perhaps the IPython config directory should be set to a temporary directory that can be deleted after the doctest is run? That would fulfill the ideal that doctests don't leave files laying around.
comment:4 Changed 9 years ago by
The doctest shouldn't leave any extra files lying around: on startup, Sage should create DOT_SAGE/ipython/
(if it doesn't exist already) and populate it with various files. From the main sage
script (SAGE_ROOT/spkg/bin/sage
):
IPYTHONDIR="$DOT_SAGE/ipython" && export IPYTHONDIR IPYTHONRC="ipythonrc" && export IPYTHONRC if [ ! -d "$IPYTHONDIR" ]; then mkdir -p "$DOT_SAGE" cp -r "$SAGE_ROOT/ipython" "$DOT_SAGE/" fi
Running the doctest shouldn't create anything new, just use the already-present config files.
comment:5 Changed 9 years ago by
- Reviewers set to Keshav Kini
- Status changed from needs_review to positive_review
Oops, I didn't realize this hadn't been reviewed! The code is obviously correct. Positive review from me.
comment:6 Changed 9 years ago by
- Status changed from positive_review to needs_info
Why not do this in sage-env
? That would seem a lot more logical to me.
comment:7 follow-up: ↓ 9 Changed 9 years ago by
I think if you run sage --ipython
, it should not use the files in DOT_SAGE/ipython
; otherwise, the default IPython prompt becomes sage:
, for example.
comment:8 Changed 9 years ago by
I agree with John, for what it's worth.
comment:9 in reply to: ↑ 7 Changed 9 years ago by
- Status changed from needs_info to needs_review
Replying to jhpalmieri:
I think if you run
sage --ipython
, it should not use the files inDOT_SAGE/ipython
; otherwise, the default IPython prompt becomessage:
, for example.
Cool. In this case, what you did is the right thing.
comment:10 Changed 9 years ago by
- Status changed from needs_review to positive_review
Why can't I directly change status from needs_info to positive_review? I always find this so annoying.
comment:11 Changed 9 years ago by
- Merged in set to sage-5.1.beta6
- Resolution set to fixed
- Status changed from positive_review to closed
So this ticket depends on #12719, right?