Opened 2 years ago

Closed 2 years ago

#29885 closed enhancement (fixed)

Remove sagenb from "sage --notebook"

Reported by: John Palmieri Owned by:
Priority: major Milestone: sage-9.2
Component: scripts Keywords:
Cc: Merged in:
Authors: John Palmieri Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: f5853f9 (Commits, GitHub, GitLab) Commit: f5853f9be456b0add1c6e9cb82f3f1f1741a5cf4
Dependencies: Stopgaps:

Status badges

Description

  • Remove sagenb as a documented argument to the sage-notebook script, and remove the code to start the SageNB server.
  • Rewrite the error when sagenb is passed as an argument.
  • Change the default from SageNBExport to NotebookJupyter.

Change History (9)

comment:1 Changed 2 years ago by John Palmieri

Branch: u/jhpalmieri/no-sagenb-in-sage-notebook

comment:2 Changed 2 years ago by John Palmieri

Commit: 62bfb51d0ffa48d4c0c80c9faf65995651002790
Status: newneeds_review

New commits:

62bfb51trac 29885: sage-notebook: remove sagenb as an option,

comment:3 Changed 2 years ago by Karl-Dieter Crisman

I am okay with this. I would just add something like the last line here:

        print('See https://wiki.sagemath.org/Python3-Switch')
        print('Use sage --notebook=export to export sagenb notebooks to Jupyter')

comment:4 Changed 2 years ago by Karl-Dieter Crisman

By the way, see #17590 and possibly elsewhere. But since 9.2 will (apparently) not support Py2, perhaps it's indeed better to simply remove sagenb than to keep it as an optional package that can't be launched from the command line.

comment:5 Changed 2 years ago by git

Commit: 62bfb51d0ffa48d4c0c80c9faf65995651002790f5853f9be456b0add1c6e9cb82f3f1f1741a5cf4

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

f5853f9trac 29885: add to the error message for "sage -n sagenb"

comment:6 Changed 2 years ago by John Palmieri

Here is an expansion of the "sage -n sagenb" error message.

comment:7 in reply to:  6 Changed 2 years ago by Karl-Dieter Crisman

Here is an expansion of the "sage -n sagenb" error message.

Thanks. The code looks good, someone who can test the branch should just confirm that this works, and that tests pass - I can imagine some stray doctest somewhere needing the class?

comment:8 Changed 2 years ago by Travis Scrimshaw

Reviewers: Travis Scrimshaw
Status: needs_reviewpositive_review

LGTM.

comment:9 Changed 2 years ago by Volker Braun

Branch: u/jhpalmieri/no-sagenb-in-sage-notebookf5853f9be456b0add1c6e9cb82f3f1f1741a5cf4
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.