Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add '--simple-prompt' argument for sage #25363

Closed
mmmm1998 mannequin opened this issue May 14, 2018 · 53 comments
Closed

Add '--simple-prompt' argument for sage #25363

mmmm1998 mannequin opened this issue May 14, 2018 · 53 comments

Comments

@mmmm1998
Copy link
Mannequin

mmmm1998 mannequin commented May 14, 2018

IPython accepts a --simple-prompt argument, changing the input handler from prompt_toolkit to raw_input.

It would be very useful if Sage could pass this argument to IPython,
as it does with -gthread, -qthread, -q4thread, -wthread and -pylab,
because in Cantor, a front-end program for mathematical packages,
the Sage backend is now broken and that would fix it.

CC: @slel @embray @jdemeyer @EmmanuelCharpentier @dimpase

Component: scripts

Keywords: Cantor, ipython

Author: John Palmieri

Branch/Commit: 3b8b2bd

Reviewer: Frédéric Chapoton

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

@mmmm1998 mmmm1998 mannequin added c: scripts labels May 14, 2018
@mmmm1998
Copy link
Mannequin Author

mmmm1998 mannequin commented May 14, 2018

Attachment: 0001-Add-new-option-for-ipython-simple-prompt.patch.gz

Format patch

@mmmm1998

This comment has been minimized.

@fchapoton
Copy link
Contributor

Branch: public/25363

@fchapoton
Copy link
Contributor

Commit: bcf0ff0

@fchapoton
Copy link
Contributor

comment:2

I have made a proper git branch.


New commits:

bcf0ff0ipython simple prompt option

@fchapoton
Copy link
Contributor

Author: Frédéric Chapoton

@slel
Copy link
Member

slel commented May 15, 2018

Changed keywords from ipython to Cantor, ipython

@slel
Copy link
Member

slel commented May 15, 2018

comment:3

It would be nice to fix the Sage backend for Cantor!

@slel

This comment has been minimized.

@slel
Copy link
Member

slel commented May 15, 2018

Changed author from Frédéric Chapoton to Nikita Sirgienko, Frédéric Chapoton

@fchapoton
Copy link
Contributor

comment:5

review, somebody ?

@mezzarobba
Copy link
Member

comment:6
$ ./sage --simple-prompt
/home/marc/co/sage/src/bin/sage: line 1067: [: missing `]'

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 26, 2018

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

e1a7676Merge branch 'public/25363' in 8.3.b7
ecf408efix

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 26, 2018

Changed commit from bcf0ff0 to ecf408e

@fchapoton
Copy link
Contributor

comment:8

oops, fixed

@embray
Copy link
Contributor

embray commented Jun 26, 2018

comment:9

This seems fine in principle, though, I would call it just -simple-prompt with a single hyphen since, although inconsistent with IPython, is at least consistent with how the existing arguments are spelled in Sage (which itself is a somewhat unfortunate historical convention). I would actually like to completely redo Sage's CLI, but priorities priorities...

Now, as for this change, other than the single hyphen thing this is fine since it's consistent with what's already done. But I hadn't looked at this bit of code before and I find it rather suspicious in the first place. I don't like that there's some hard-coded list of arguments that happen to be arguments recognized as belonging to IPython in order to determine that the user is trying to invoke an interactive prompt, because as you've pointed out here it does not scale--it's just an arbitrary subset of IPython arguments, and does not inherit new IPython arguments or additional ones that one might want to use. Let me take a closer look at this problem and think about it a bit...

@mezzarobba
Copy link
Member

comment:10

The --simple-prompt option now works, but this does not seem to be enough to fix the Sage backend in Cantor. (On my system, the backend now starts, but it still fails to evaluate expressions.)

@embray
Copy link
Contributor

embray commented Jun 26, 2018

comment:11

In fact, with the exception of --pylab (which itself is deprecated) most of those options don't work anymore:

$ ./sage -gthread
...
[SageTerminalApp] CRITICAL | Unrecognized flag: '-gthread'

@embray
Copy link
Contributor

embray commented Jun 26, 2018

comment:12

How does the Sage backend in Cantor work? Is it like, pexpect-based or something? I would suggest that it not be. Sage is, ultimately, a Python library, so there's no need to use it through Sage's interactive prompt, in order to pass input and receive output. That said, I have no idea how Cantor's backends work in general.

@embray
Copy link
Contributor

embray commented Jun 26, 2018

Changed branch from public/25363 to u/embray/ticket-25363

@embray
Copy link
Contributor

embray commented Jun 26, 2018

comment:13

This preserves the existing (still rather kludgy) CLI semantics, while not hard-coding a limited subset of IPython CLI options. This means you can also pass --simple-prompt which works as expected.


New commits:

0bb9b1aPython 3 fixes for sage.misc.decorators.
19f3d11dont' try to make sage_wraps work on classes at all
1f61a71for compatibility with Python 3's functools.wraps, set `__wrapped__` attribute on the wrapper function
44ee2ecclean up the infix_operator docstrings a bit to demonstrate actual use as a decorator
49f44efslight changes to (still kludgy) command line handling
237cbcbAlso update the docs accordingly

@videlec
Copy link
Contributor

videlec commented Aug 3, 2018

comment:26

update milestone 8.3 -> 8.4

@videlec videlec modified the milestones: sage-8.3, sage-8.4 Aug 3, 2018
@mkoeppe mkoeppe modified the milestones: sage-8.4, sage-9.2 Aug 11, 2020
@jhpalmieri
Copy link
Member

comment:28

I looked at the old commits, and I am concerned that they are too ambitious: they would pass any unrecognized command-line flag to ipython. Here is a more modest proposal:

diff --git a/src/bin/sage b/src/bin/sage
index ce325f5d03..bc192ab6e5 100755
--- a/src/bin/sage
+++ b/src/bin/sage
@@ -1045,7 +1045,7 @@ if [ "$1" = '-omega' -o "$1" = "--omega" ]; then
     exec sage-omega "$@"
 fi
 
-if [ "$1" = '-gthread' -o "$1" = '-qthread' -o "$1" = '-q4thread' -o "$1" = '-wthread' -o "$1" = '-pylab' ]; then
+if [ "$1" = '-gthread' -o "$1" = '-qthread' -o "$1" = '-q4thread' -o "$1" = '-wthread' -o "$1" = '-pylab' -o "$1" = '--simple-prompt' -o "$1" = '-simple-prompt' ]; then
     # Intentionally no "shift" here
     interactive_sage "$@"
 fi

(The new option should also be listed in the help message.)

@jhpalmieri
Copy link
Member

Changed branch from u/embray/ticket-25363 to u/jhpalmieri/ticket-25363

@jhpalmieri
Copy link
Member

Changed author from Erik Bray to John Palmieri

@jhpalmieri
Copy link
Member

Changed commit from ed2d023 to 3b8b2bd

@jhpalmieri
Copy link
Member

comment:30

Here is an actual branch. The more ambitious changes can happen on another ticket, but we should get this done quickly to fix the emacs problem. (Any problem with emacs has a critical priority, at least.)


New commits:

3b8b2bdtrac 25363: implement "sage --simple-prompt"

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Aug 11, 2020

comment:31

Thank you, John.

Now, the question is to adopt your patch or to use it as a stopgap until more ambitious (and more reasoned) changes to Sage-s arguments implement long-awaited features can be (carefully) validated...

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Aug 11, 2020

comment:32

Replying to @jhpalmieri:

[ Snip... ] (Any problem with emacs has a critical priority, at least.)

Seconded. With extreme energy...

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Aug 11, 2020

comment:33

Replying to @jhpalmieri:

Here is an actual branch. The more ambitious changes can happen on another ticket, but we should get this done quickly to fix the emacs problem. (Any problem with emacs has a critical priority, at least.)


New commits:

3b8b2bdtrac 25363: implement "sage --simple-prompt"

OK. It does what the label says : the sage-shell-mode session is functional again. Now running ptestlong by principle.

Stay tuned.

@jhpalmieri
Copy link
Member

comment:34

Replying to @EmmanuelCharpentier:

Thank you, John.

Now, the question is to adopt your patch or to use it as a stopgap until more ambitious (and more reasoned) changes to Sage-s arguments implement long-awaited features can be (carefully) validated...

See also #21.

@mkoeppe

This comment has been minimized.

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Aug 11, 2020

comment:36

Replying to @jhpalmieri:

See also #21.

Both typesetting and plots work (tested in simple cases...).

I'm currently scratching my head to figure how to patch sage-shell-mode (and possibly ob-sagemath to support Sage>9.2.beta8 without loosing support for Sage<=9.2.beta7 (9.2.beta8 itself is a lost cause...). While ptestlong runs (and heats the cosmos...).

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Aug 11, 2020

comment:37

Replying to @EmmanuelCharpentier:

Replying to @jhpalmieri:

See also #21.

Both typesetting and plots work (tested in simple cases...).

I'm currently scratching my head to figure how to patch sage-shell-mode (and possibly ob-sagemath to support Sage>9.2.beta8 without loosing support for Sage<=9.2.beta7 (9.2.beta8 itself is a lost cause...). While ptestlong runs (and heats the cosmos...).

However, quit() is no longer enough to close Sage : emacs hangs after printing Process Sage interrompre two lines below the Exiting Sage (CPU time 0m12.27s, Wall time 20m30.50s). message.

Back to the old drawing board...

@jhpalmieri
Copy link
Member

comment:38

Emacs does not hang for me after running quit(). I could switch to another buffer, back to the Sage buffer, and run Sage again:

sage: quit()
Exiting Sage (CPU time 0m0.47s, Wall time 0m24.00s).

Process Sage finished
┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 9.2.beta8, Release Date: 2020-08-10               │
│ Using Python 3.7.3. Type "help()" for help.                        │
└────────────────────────────────────────────────────────────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Warning: this is a prerelease version, and it may be unstable.     ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
sage: 3+5
8
sage: 

This also worked after I turned off my .emacs file (the moral equivalent of emacs -q). Could something in your emacs setup be interfering?

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Aug 11, 2020

comment:39

Replying to @EmmanuelCharpentier:

[ ... ]

However, quit() is no longer enough to close Sage : emacs hangs after printing Process Sage interrompre two lines below the Exiting Sage (CPU time 0m12.27s, Wall time 20m30.50s). message.

Back to the old drawing board...

I was a tad pessimistic : emacs doesn't really hangs, and can be left by C-x C-c, without need for kill.

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Aug 11, 2020

comment:40

This passes ptestlong with results identical to those already reported since 9.2.beta5.

==>positive_review

@vbraun
Copy link
Member

vbraun commented Aug 14, 2020

Changed branch from u/jhpalmieri/ticket-25363 to 3b8b2bd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants