@options() decorators in sagemath library code and Sphinx 7.1+

[Dima Pasechnik]

We have instances of @options() (from src/sage/misc/decorators.py)
applied to functions in the library, e.g.
https://github.com/sagemath/sage/blob/41031292ff1ae518cd5b5a29ce277aa1ff8ced9e/src/sage/plot/contour_plot.py#L1394

where an argument name of the form foo=bar in @options
matches an argument name foo of the function, e.g.

@options(plot_points=100, incol='blue', outcol=None, bordercol=None,
borderstyle=None, borderwidth=None, frame=False, axes=True,
legend_label=None, aspect_ratio=1, alpha=1)
def region_plot(f, xrange, yrange, plot_points, incol, outcol, bordercol,
borderstyle, borderwidth, alpha, **options):

Sphinx 7.1 (or newer) does not like these, it complains about duplicate names.

Isn't it true that the above may be simplified, removing foo=bar from
@options() and putting this in the function definition, i.e.

@options(frame=False, axes=True,
legend_label=None, aspect_ratio=1, alpha=1)
def region_plot(f, xrange, yrange, plot_points=100, incol='blue',
outcol=None, bordercol=None,
borderstyle=None, borderwidth=None, alpha=1, **options):

would result in the same functionality of region_plot() ?
(Asssuming in the function body no need to have
options['plot_points'], etc. arises?)

Dima

[Michael Orlitzky]

On Sat, 2023-09-16 at 13:16 +0100, Dima Pasechnik wrote:
> Isn't it true that the above may be simplified, removing foo=bar from
> @options() and putting this in the function definition, i.e.
>

AFAIK it's just dictionary unpacking that happens by default. You can
already create your own options list for functions that take keyword
arguments,

sage: def foo(x=None, y=None):
....: print(x, y)
....:
sage: foo_options = {"x": "hello,", "y": "world!"}
sage:
sage: foo(**foo_options)
hello, world!

but you have to remember to pass the options yourself whenever you call
foo().

[Dima Pasechnik]

Yes - there is also an ugly mix of positional and key argments happening here.

Anyhow, if this is the only problem with upgrading to Sphinx 7.1+ (or 8)
it ought to be fixed, so that we can move on on Sphinx update.
(see https://github.com/sagemath/sage/pull/35658)

Dima
>
> --
> You received this message because you are subscribed to the Google Groups "sage-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/d2a5ce1d8edbd0b971bf048f6d5afffef5f72250.camel%40orlitzky.com.

[Michael Orlitzky]

On 2023-09-16 14:09:04, Dima Pasechnik wrote:
>
> Anyhow, if this is the only problem with upgrading to Sphinx 7.1+ (or 8)
> it ought to be fixed, so that we can move on on Sphinx update.
> (see https://github.com/sagemath/sage/pull/35658)

My vote would be to replace the weird sageism with the standard python
construct, but that would silently break a lot of plots. A deprecation
warning would be nice, but if there's no workaround for sphinx, then
letting our sphinx get N+1 years out-of-date is not very attractive
either. Maybe there's a middle ground that warns people who are
actually using the implicit options to migrate and inspect their
plots.