Notebook commandline argparse

[vbraun]

• Do not require quotes in sage --notebook sagenb foo=bar

• Handle -- to finish options: sage --notebook -- foo=bar only passes foo=bar to sagenb and does not treat it as the value for --notebook

[git]

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

​ce9db90

Do not pass "--" option on to notebook

[kcrisman]

Is there a way to doctest this parsing? If not, it's okay, just asking. Thanks for opening this.

[vbraun]

The doctesting framework doesn't look into src/bin/...

[kcrisman]

Oh, I meant in another file, perhaps in the notebook (though that is also hard to change quickly, sigh for it being this separate project...)

[vbraun]

You can start a bunch of notebook servers with different options, but imho thats dog slow. The right way to do it is to look at doctests in src/bin scripts.

[kcrisman]

That may be true.

But what I meant was just something that tested the parsing of the options, as a guard against when someone someday changes that to deal with another special case. That presumably is something that could be done, but maybe not without testing the scripts. Anyway, if it's not easy I don't want to hold this up - my apologies that I can't test it right now.

[vbraun]

Once more: Not with the current doctest framework. Sure, there are all kinds of ugly hacks that one could do. But there is only one way to do it right, and we aren't set up for it.

[kcrisman]

From sage-release, a suggestion to get this finalized:

---

• whether the option --notebook-dir is documented in ./sage -h (or --advanced); IMHO it should be

It is not, and that points us to

$ ./sage --notebook --help
usage: sage-notebook [-h] [--log LOG] [--notebook [NOTEBOOK]]

The Sage notebook launcher is used to start the notebook, and allows you to
choose between different implementations. Any further command line options are
passed to the respective notebook.

optional arguments:
  -h, --help            show this help message and exit. Can be combined with
                        "--notebook=[...]" to see notebook-specific options
  --log LOG             one of [DEBUG, INFO, ERROR, WARNING, CRITICAL]
  --notebook [NOTEBOOK], -n [NOTEBOOK], -notebook [NOTEBOOK]
                        The notebook to run [one of: default, ipython,
                        sagenb]. Default is sagenb

which also doesn't help, while that points us to

$ ./sage --notebook=sagenb --help

which unfortunately just gives the notebook? commands but doesn't tell how to use them with the notebook-specific options.

• on whether the old behaviour "-n directory=sage.sagenb" is documented somewhere and whether this documentation has been adapted to the new situation.

It was documented in the following way, in sage --advanced

  -n, -notebook [...] -- start the Sage notebook (options are the same
                         as for the notebook command in Sage)

So I would say that since I do agree that anyone using this option probably is savvy enough to actually read the help, perhaps documenting exactly how the options are to be called in sage --advanced and/or sage --notebook --help would be enough to resolve this issue. (Changing sage --notebook=sagenb --help would require more annoyance than worth.) This should be pretty easy to do in the script itself, and given that we never said exactly how to *call* the options before I think that as long as we clearly document this (not necessarily with zillions of examples, but maybe two) then this would work okay.

One could at the same time add the info about the default directory, incidentally.

[vbraun]

Replying to kcrisman:

which unfortunately just gives the notebook? commands but doesn't tell how to use them with the notebook-specific options.

Which part of "Any further command line options are passed to the respective notebook" is unclear?

If you want to add to the online-help then just post a commit. IMHO it is clear and concise. There is always a tradeoff between longer and buried in a text wall.

[kcrisman]

which unfortunately just gives the notebook? commands but doesn't tell how to use them with the notebook-specific options.

Which part of "Any further command line options are passed to the respective notebook" is unclear?

Because user X will know to read help, but may NOT understand the arcane syntax for doing so! Namely, I would think this continues to allow

sage --notebook foo=bar

because I had never even heard of the sage --notebook -- foo=bar syntax. Yes, I'm stupid and uneducated. Well, so will many users be. Especially since I was wrong - I said

It was documented in the following way,

but it still is documented this way.

If you want to add to the online-help then just post a commit. IMHO it is clear and concise. There is always a tradeoff between longer and buried in a text wall.

Yes, of course. Though Let me see if I can cook something up; my concern is that I will make a stupid mistake, while you speak this syntax natively, as it were. For instance, is this a way to spice this up correctly?

usage: sage-notebook [-h] [--log LOG] [--notebook [NOTEBOOK]] [--option=foo ...]

The Sage notebook launcher is used to start the notebook, and allows you to
choose between different implementations. Any further command line options are
passed to the respective notebook.

optional arguments:
  -h, --help            show this help message and exit. Can be combined with
                        "--notebook=[...]" to see notebook-specific options
  --log LOG             one of [DEBUG, INFO, ERROR, WARNING, CRITICAL]
  --notebook [NOTEBOOK], -n [NOTEBOOK], -notebook [NOTEBOOK]
                        The notebook to run [one of: default, ipython,
                        sagenb]. Default is sagenb}}}
   --option=foo       Additional options to be sent to the notebook can
                               be added using this syntax

[vbraun]

Argparse auto-generates the help from the available options, you can't make options appear in the online help without handling them.

[kcrisman]

Oh.

BUT we do have

usage_advanced() {
....
}

and at the very least

    echo "  -n, -notebook [...] -- start the Sage notebook (options are the same"
    echo "                         as for the notebook command in Sage)"

could be fixed somehow. From looking at src/bin/sage it looks like this only runs the default notebook.

And would this be acceptable?

description = \
"""
The Sage notebook launcher is used to start the notebook, and allows
you to choose between different implementations. Any further command
line options, in the form `--option=foo`, are passed to the respective notebook.
"""

[vbraun]

Yes, you can put anything in the description.

[kcrisman]

Yes, you can put anything in the description.

Okay, I'll do that. But is it correct? That's what I meant, sorry. I'll do that and sage --advanced and that should resolve the last bit here, assuming someone looks at and tests your actual change.

[vbraun]

It doesn't have to have the form --option=foo, anything else is passed to the notebook. E.g. port=666 without dashes. How about just giving two examples

sage -n default port=1234                              # pass port=1234 to default notebook
sage --notebook=ipython --notebook-dir=/home/foo/bar   # ipython notebook in custom dir

[kcrisman]

Thanks, I'll do something along those lines (was working on it but then was interrupted by something high-priority). I think the point was that one needed --option=foo if one didn't say which type of notebook, right?

[kcrisman]

I don't want to commit until I'm sure I'm following standards for man-type help... here is a potential commit.

src/bin/sage

@@ -72 +72 @@
     ####  1.......................26..................................................78
     ####  |.....................--.|...................................................|
     echo "Running the notebook:"
-    echo "  --notebook=[...]    -- start the Sage notebook (valid options are"
-    echo "                         'default', 'sagenb', and 'ipython'). See the output"
-    echo "                         'of sage --notebook --help for more details"
+    echo "  --notebook[=default] [...] -- start the Sage notebook (valid options are"
+    echo "                         'default', 'sagenb', and 'ipython'). Notebook-specific"
+    echo "                         options are passed on; if notebook is not specified, they"
+    echo "                         must be in the form '--option=foo' or '-- option=foo'."
+    echo "                         See sage --notebook --help for more details"
     echo "  -bn, -build-and-notebook [...] -- build the Sage library then start"
     echo "                         the Sage notebook"
     echo "  -inotebook [...]    -- start the *insecure* Sage notebook (deprecated)"
-    echo "  -n, -notebook [...] -- start the Sage notebook (options are the same"
-    echo "                         as for the notebook command in Sage)"
+    echo "  -n, -notebook [...] -- start the default Sage notebook.  (Options are the same"
+    echo "                         as for the notebook command in Sage, passed in the form"
+    echo "                         'option=foo'.)"
 
     echo
     ####  1.......................26..................................................78

src/bin/sage-notebook

@@ -77 +77 @@
 The Sage notebook launcher is used to start the notebook, and allows
 you to choose between different implementations. Any further command
 line options are passed to the respective notebook.
+
+Examples:
+
+sage -n default port=1234                              # pass port=1234 to default notebook
+sage --notebook=ipython --notebook-dir=/home/foo/bar   # ipython notebook in custom dir
 """
 
 help_help = \

[cheuberg]

I tested a few things.

Does not work as expected:

• Directory with slashes

  ./sage -n default directory=/tmp/test
  Traceback (most recent call last):
  ...
    File "/local/sage/sage-6.4.rc1/local/lib/python/ast.py", line 37, in parse
      return compile(source, filename, mode, PyCF_ONLY_AST)
    File "<unknown>", line 1
      /tmp/test
      ^
  SyntaxError: invalid syntax
  

  This worked in 6.3:

  /local/sage/sage-6.3/sage -n directory=/tmp/test
  

Work as expected:

./sage -n
./sage -n default
./sage -n -- directory=test
./sage -n default directory=test.sagenb
./sage -n sagenb "directory='/tmp/test'"
./sage -n -- "directory='/tmp/test'"
./sage -n default port=1234
./sage -n -- port=1234

Replying to kcrisman:

I think the point was that one needed --option=foo if one didn't say which type of notebook, right?

I do not see what you mean here; I could not get any of --directory=test or --option=directory=test to work, but I did not really expect it previously; but as I said, I do not see what you mean with --option=foo.

[git]

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

​f868e97	also catch SyntaxError when parsing
​1baf6fe	add examples

[vbraun]

Thanks, fixed. I also added the examples:

$ ./sage -n -h
usage: sage-notebook [-h] [--log LOG] [--notebook [NOTEBOOK]]

The Sage notebook launcher is used to start the notebook, and allows
you to choose between different implementations. Any further command
line options are passed to the respective notebook.

optional arguments:
  -h, --help            show this help message and exit. Can be combined with
                        "--notebook=[...]" to see notebook-specific options
  --log LOG             one of [DEBUG, INFO, ERROR, WARNING, CRITICAL]
  --notebook [NOTEBOOK], -n [NOTEBOOK], -notebook [NOTEBOOK]
                        The notebook to run [one of: default, ipython,
                        sagenb]. Default is sagenb

EXAMPLES: 

* Run default notebook on port 1234. Note that the first argument
  after "-n" will be interpreted as notebook name unless you stop
  processing with "--":

      sage -n default port=1234
      sage -n -- port=1234      # equivalent
      sage -n port=1234         # ERROR: invalid notebook name

* Run IPython notebook in custom directory:

      sage --notebook=ipython --notebook-dir=/home/foo/bar

[kcrisman]

Thanks for the feedback, Clemens, that's very helpful. I checked that this fixed it, and it certainly shouldn't break anything.

Volker - oh, that makes great sense, and I like the way you did it. I hope the following reviewer patch is acceptable for src/bin/sage.

---

New commits:

​4a0fc71

Add details to sage --advanced