Allow specifying arguments to Cython cell_magic

[user202729]

As seen in the code, this allows user to write code like

%%cython --verbose=1
def f():
    print(1)

and have the verbose=1 passed to Cython compilation function.

📝 Checklist

• The title is concise and informative.
• The description explains in detail what this PR is about.
• I have linked a relevant issue or discussion. (not aware of one)
• I have created tests covering the changes.
• I have updated the documentation and checked the documentation preview.

---

Test failure is unrelated https://github.com/sagemath/sage/issues/33906 (fixed in newest version)

[github-actions]

Documentation preview for this PR (built with commit 531b1d7; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

[kwankyu]

LGTM.

[user202729]

Actually, looking at e.g. /IPython/core/magics/execution.py, the convention is to use IPython.core.magic.Magics.parse_options, which has a getopt.getopt()-like interface.

Is it worth it to change it to be consistent?

On the other hand getopt documentation says

Deprecated since version 3.13: The getopt module is soft deprecated and will not be developed further; development will continue with the argparse module.

and IPython parse_options uses getopt internally.

(are you sure you don't mean to approve the other PR and leave the comment on documentation on this one instead?)

[kwankyu]

Actually, looking at e.g. /IPython/core/magics/execution.py, the convention is to use IPython.core.magic.Magics.parse_options, which has a getopt.getopt()-like interface.

Is it worth it to change it to be consistent?

On the other hand getopt documentation says

Deprecated since version 3.13: The getopt module is soft deprecated and will not be developed further; development will continue with the argparse module.

and IPython parse_options uses getopt internally.

I think we should be future-proof and argparseis the future.

(are you sure you don't mean to approve the other PR and leave the comment on documentation on this one instead?)

Either way is okay with me. Just let me know which one depends on which.

[kwankyu]

For our simple arguments, are there differences in argument style between getopt and argparse?

[user202729]

On the user side, it would looks mostly the same. But the code would look like this (i.e. more manual handling)

        boolean_options = {
                "m": "compile-message",
                "c": "use-cache",
                "l": "create-local-c-file",
                "a": "annotate",
                "s": "sage-namespace",
                "o": "create-local-so-file",
                }
        args, rest = self.parse_options(line, "v:" + "".join(boolean_options), "verbose=",
                                              *boolean_options.values(),
                                              *["no-" + k for k in boolean_options.values()])
        from IPython.core.error import UsageError
        if rest:
            raise UsageError(f"unrecognized arguments: {rest}")
        args_dict = dict(args)
        if "v" in args_dict:
            args_dict["verbose"] = args_dict["v"]
            del args_dict["v"]
        if "verbose" in args_dict:
            args_dict["verbose"] = int(args_dict["verbose"])
        for k in [*args_dict]:
            if k in boolean_options:
                args_dict[boolean_options[k]] = True
                del args_dict[k]
            elif k in boolean_options.values():
                args_dict[k] = True
            elif k.startswith("no-"):
                args_dict[k[3:]] = False
                del args_dict[k]
        args_dict = {k.replace("-", "_"): v for k, v in args_dict.items()}
        return cython_compile(cell, **args_dict)

or

        args, rest = self.parse_options(
                line, "v:mclaso", "verbose=",
                "compile-message", "use-cache", "create-local-c-file", "annotate", "sage-namespace", "create-local-so-file",
                "no-compile-message", "no-use-cache", "no-create-local-c-file", "no-annotate", "no-sage-namespace", "no-create-local-so-file",
                )
        from IPython.core.error import UsageError
        if rest:
            raise UsageError(f"unrecognized arguments: {rest}")
        args_dict = {}
        for k, v in args.items():
            if k in ("v", "verbose"):
                args_dict["verbose"] = int(v)
            elif k in ("m", "compile-message"):
                args_dict["compile_message"] = True
            elif k in ("c", "use-cache"):
                args_dict["use_cache"] = True
            elif k in ("l", "create-local-c-file"):
                args_dict["create_local_c_file"] = True
            elif k in ("a", "annotate"):
                args_dict["annotate"] = True
            elif k in ("s", "sage-namespace"):
                args_dict["sage_namespace"] = True
            elif k in ("o", "create-local-so-file"):
                args_dict["create_local_so_file"] = True
            elif k.startswith("no-"):
                args_dict[k[3:].replace("-", "_")] = False
            else:
                raise UsageError(f"unrecognized argument: {k}")
        return cython_compile(cell, **args_dict)

Only the error message would look different

sage: %%cython -v1 --annotate --no-sage-namespace --xx
....: def f():
....:     print('test')
....: 
UsageError: option --xx not recognized ( allowed: "v:mclaso" verbose= compile-message use-cache crea
te-local-c-file annotate sage-namespace create-local-so-file no-compile-message no-use-cache no-crea
te-local-c-file no-annotate no-sage-namespace no-create-local-so-file)

which aligns with existing IPython magics

sage: %run -abc
UsageError: option -a not recognized ( allowed: "nidtN:b:pD:l:rs:T:em:G" )

[kwankyu]

I see. Aligning with existing ipython magics is good for consistency. Consistency is more visible to users. Hence I would value consistency more than future-proof. Anyway, I will leave the decision to you.

[user202729]

I think there's also the part that using parse_arguments() instead of argparse makes the code significantly more complicated. (there's the BooleanOptionalAction thing which makes it even shorter but is Python 3.9 only.)

I don't think consistency is a big issue, now that I raise UsageError instead of argparse.ArgumentError.

[kwankyu]

Done? LGTM.

[user202729]

Done? LGTM.

Can you change the label to positive-review then? Thanks.

---

(not to try to bikeshedding the issue, but: do you think it's a good idea to specify all short options like that? It may take up space for other more sensical options later. I'd say -v and -a is obvious, but others maybe not so much… Later maybe we could use -w for view-annotate (what's the logic) and either -p for preparse or -s for sage-preparse (because of the pyx versus spyx extension).)

[kwankyu]

not to try to bikeshedding the issue,

Setting up a bike shed in a lot beside of a nuclear powerplant is perhaps a trivial thing. But in software engineering, I think small things are not trivial; they affect many people for a long time.

do you think it's a good idea to specify all short options like that?

Alternatively, you may just give a couple of examples with useful options.

[kwankyu]

Will you merge this branch to #38946?

[kwankyu]

When you are done with this PR, you may set "positive review".

[user202729]

will merge later when it's stable.

No, what I mean is should short option exists for every long option --- for example -s for --sage-namespace might not make sense, and later we may want -s to stand for e.g. --sage-preparse instead which is admittedly more common.

Problem is once a short option is taken for something it cannot be taken for something else (which in retrospect might make more sense later) without like a year of deprecation so…

I don't think I have permission to set review labels myself.

[kwankyu]

will merge later when it's stable.

OK.

No, what I mean is should short option exists for every long option --- for example -s for --sage-namespace might not make sense, and later we may want -s to stand for e.g. --sage-preparse instead which is admittedly more common.

Problem is once a short option is taken for something it cannot be taken for something else (which in retrospect might make more sense later) without like a year of deprecation so…

I see. I agree that short option should exist only for frequent long options.

I don't think I have permission to set review labels myself.

OK. Let me know when you are done.

[user202729]

I realize I don't know which long option are common… but these 2 are definitely not common (they defaults to True).

Chances are it doesn't matter that much anyway. I'm done with this.