An alternative for function objects in configuration parameters?

[Matthias Geier]

Dear list.

I've just found out that when I'm using a function object in a
configuration parameter in conf.py, this effectively disables the
Sphinx mechanism for caching the build environment. Repeated calls to
Sphinx will always build all source files, even if nothing has
changed.

I've found an issue about that topic, including the recommendation by
Takeshi KOMIYA that functions should not be used in config parameters:
https://github.com/sphinx-doc/sphinx/issues/7093#issuecomment-581484955

My problem is that I already have defined a configuration parameter in
my Sphinx extension that expects a function:
https://nbsphinx.readthedocs.io/en/0.5.1/custom-formats.html

In some cases this is fine because a function can be given as a
string, but only if this function takes exactly one parameter:

nbsphinx_custom_formats = {
'.mysuffix': 'mylibrary.converter_function',
}

In other cases (and I guess those are the most realistic cases), the
function selected by the user takes some more parameters (mostly
keyword parameters, I guess), and the user can use a lambda function
(or local function) to handle those. For example:

nbsphinx_custom_formats = {
'.Rmd': lambda s: jupytext.reads(s, '.Rmd'),
}

Does anyone have an idea how I could solve that without using function objects?

I guess the user could specify a string for the function name (e.g.
'jupytext.reads') plus a dictionary for keyword arguments (e.g.
{'fmt': '.Rmd'}), which could look something like this:

nbsphinx_custom_formats = {
'.Rmd': ['jupytext.reads', dict(fmt='.Rmd')],
}

Would this make sense?

Does anyone have a better idea?

cheers,
Matthias

[Komiya Takeshi]

Hi Mattias,

I think it is better to use `setup()` function on conf.py to enhance
the feature. It's like a small extension.

Thanks,
Takeshi KOMIYA

2020年3月12日(木) 0:10 Matthias Geier <matthia...@gmail.com>:

> --
> You received this message because you are subscribed to the Google Groups "sphinx-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to sphinx-dev+...@googlegroups.com.
> To view this discussion on the web, visit https://groups.google.com/d/msgid/sphinx-dev/CAFesC-crWf%2BHGasn6cEAexRcuQMKUVSBBMb3Zmh7asAt61LBNQ%40mail.gmail.com.

[Matthias Geier]

Thanks for the answer Takeshi!

... but I don't understand what you mean.

On Sun, Mar 15, 2020 at 12:28 PM Komiya Takeshi wrote:
>
> I think it is better to use `setup()` function on conf.py to enhance
> the feature. It's like a small extension.

Do you mean that users of my extension should use a setup() function
in their conf.py to configure my extension?

I don't understand how this is supposed to work, can you please clarify?

BTW, I've made a PR with a possible implementation
(https://github.com/spatialaudio/nbsphinx/pull/408), but I'm still
open to suggestions for better ways to do this.

cheers,
Matthias

> Thanks,
> Takeshi KOMIYA
>
> 2020年3月12日(木) 0:10 Matthias Geier:

[Komiya Takeshi]

Sorry for confusing you.

I thought it's nice if your extension provides APIs to enhance the
feature. Surely, Sphinx does not expect to store codes to the config
object. But you can enhance your extension by code instead of
configuration.

For example, the following function `add_custom_format()` is a small
and simple API to install user's custom reader for an arbitrary
format.
```
# in nbsphinx/__init__.py
custom_formats = {}

def add_custom_format(suffix: str, reader: Callable) -> None:
custom_formats[suffix] = reader
```

And users can install their custom reader via their conf.py:
```
# in conf.py
def setup(app):
import nbsphinx
nbsphinx.add_custom_format('.Rmd', lambda s: jupyter.reads(s, '.Rmd'))
```

I don't know this is easy to understand for users. But I prefer this
way than representing a function call by configurations.

Thanks,
Takeshi KOMIYA

2020年3月16日(月) 1:41 Matthias Geier <matthia...@gmail.com>:

> --
> You received this message because you are subscribed to the Google Groups "sphinx-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to sphinx-dev+...@googlegroups.com.

> To view this discussion on the web, visit https://groups.google.com/d/msgid/sphinx-dev/CAFesC-c_c86zUoRRA9euaUCGWRWYNCTApoX8LXY6AUZy%2BVgxqQ%40mail.gmail.com.

[Matthias Geier]

Thanks, now I understand what you mean!

On Mon, Mar 16, 2020 at 3:37 PM Komiya Takeshi wrote:
>
> Sorry for confusing you.
>
> I thought it's nice if your extension provides APIs to enhance the
> feature. Surely, Sphinx does not expect to store codes to the config
> object.

I found one config parameter which is actually supposed to be a
function (but specified as a string):
https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-template-bridge

So I guess it wouldn't be too exotic if my extension also allows
specifying a function as a string.

The "template_bridge" parameter is simpler, however, because it
doesn't expect any arguments.

> But you can enhance your extension by code instead of
> configuration.
>
> For example, the following function `add_custom_format()` is a small
> and simple API to install user's custom reader for an arbitrary
> format.
> ```
> # in nbsphinx/__init__.py
> custom_formats = {}
>
> def add_custom_format(suffix: str, reader: Callable) -> None:
> custom_formats[suffix] = reader
> ```

I don't think this is a good Python style.

It's basically a somewhat disguised form of monkey-patching, isn't it?

I wouldn't mind doing that in a one-off personal project, but I would
like to avoid something like this in a project that other people are
supposed to use.

I think it would work perfectly fine, it's just a matter of somewhat bad style.

> And users can install their custom reader via their conf.py:
> ```
> # in conf.py
> def setup(app):
> import nbsphinx
> nbsphinx.add_custom_format('.Rmd', lambda s: jupyter.reads(s, '.Rmd'))
> ```

I think the majority of "normal" users should never define a setup()
function in their conf.py.
I think this is an advanced feature that's really nice to have for
some exotic circumstances, but I wouldn't want to suggest it to my
users for a "normal" feature.

Also, I guess it wouldn't work with Sphinx's caching mechanism, would it?

If a user would change the implementation of their setup() function,
Sphinx wouldn't re-build, right?

> I don't know this is easy to understand for users.

I think having to create a setup() function is generally harder to
understand than assigning something to a configuration value.
I think "normal" users shouldn't worry at all about a setup() function.

> But I prefer this
> way than representing a function call by configurations.

Well, I'm not sure.
Having a function object as a config parameter doesn't work nicely
w.r.t. caching, but I think it would be easy to understand.
Having a dictionary with multi-element values as a config parameter is
a bit complicated. But Sphinx also uses that, e.g.
https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-html-sidebars.
Having one of these elements be a dictionary is even more complicated:

nbsphinx_custom_formats = {
'.Rmd': ['jupytext.reads', {'fmt': '.Rmd'}],
}

A dictionary containing a list containing a dictionary is quite bad,
but I couldn't come up with a better idea until now ...

Do you have any ideas that don't involve the setup() function?

> To view this discussion on the web, visit https://groups.google.com/d/msgid/sphinx-dev/CAFmkQAM2%3Dc-Dzn6g4_AYWBoJBZDi90cihvCFvcj%3DdNwqUSrkPg%40mail.gmail.com.

[Komiya Takeshi]

> I found one config parameter which is actually supposed to be a
> function (but specified as a string):
> https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-template-bridge

If I could re-implement it at present, I'll provide an API like
`app.add_template_bridge()` for such purpose. IMO, Sphinx extensions
are better to enhance Sphinx by code, not a configuration. In this
case, TemplateBridge is a python code. So no reason not to release it
an extension.

> I don't think this is a good Python style.
>
> It's basically a somewhat disguised form of monkey-patching, isn't it?

Really? If so, Sphinx is a large amount of monkey-patching. I think
providing API is not monkey-patching.

As commented above, providing APIs are appropriate way to enhance the
programs by code, I think. And I consider the your idea; dictionary
styled nbsphinx_custom_formats is a kind of code. It tries to
represent code as data. It likes an idea of S-expressions.

I agree my idea is not beautiful and simple. But I think it is
difficult to do that simply because what you'd like to do is a
programming, not a configuration.

> Also, I guess it wouldn't work with Sphinx's caching mechanism, would it?
>
> If a user would change the implementation of their setup() function,
> Sphinx wouldn't re-build, right?

Yes. It is hard to detect the change of the implementation.

But is it available on your idea? If I set my custom function to
`nbsphinx_custom_formats`, it is also hard to detect the change of the
implementation.

nbsphinx_custom_formats = {
'.Rmd': ['mymodule.reads', {'fmt': '.Rmd'}],
}

Sphinx can only detects the change of configuration value, not the
change of the target function.

> > I don't know this is easy to understand for users.
>
> I think having to create a setup() function is generally harder to
> understand than assigning something to a configuration value.
> I think "normal" users shouldn't worry at all about a setup() function.

If my understanding correct, "normal" users you said are able to write
python code, right?
Because I saw a lambda function in your first post (I don't know it is
already implemented or not).
If so, I don't think it is not difficult way to me.

If not, I thought using configuration is also difficult for "normal"
users. I think it is better to simplify it by lessening feature.

FWIW: recommonmark explains the way to enhance it via setup()
function. I don't think this is a better way. But it's one of example
for you.
https://recommonmark.readthedocs.io/en/latest/#autostructify

> Do you have any ideas that don't involve the setup() function?

Sorry, I have no idea more.

Thanks,
Takeshi KOMIYA

[Matthias Geier]

On Sat, Mar 21, 2020 at 9:18 AM Komiya Takeshi wrote:
>
> > I found one config parameter which is actually supposed to be a
> > function (but specified as a string):
> > https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-template-bridge
>
> If I could re-implement it at present, I'll provide an API like
> `app.add_template_bridge()` for such purpose.

OK, that makes sense.
This seems to be an "expert" setting, I guess it would be mainly used
from extensions.

> IMO, Sphinx extensions
> are better to enhance Sphinx by code, not a configuration. In this
> case, TemplateBridge is a python code. So no reason not to release it
> an extension.

Yes, that sounds reasonable.

But my case is different.
The option I'm talking about is not supposed to be selecting some code
that the user wrote.
Instead, it is supposed to import an existing module and use that,
without writing any code.

This is my example again:

nbsphinx_custom_formats = {
'.Rmd': ['jupytext.reads', {'fmt': '.Rmd'}],
}

In this case, "jupytext.reads" is an already existing function, see:
https://jupytext.readthedocs.io/en/latest/using-library.html#reading-notebooks-from-many-text-formats

The user is not expected to write any code here.

> > I don't think this is a good Python style.
> >
> > It's basically a somewhat disguised form of monkey-patching, isn't it?
>
> Really? If so, Sphinx is a large amount of monkey-patching.

No, Sphinx does it differently.

For example in the setup() function we get an "app" object which the
function is allowed to manipulate.

But we don't manipulate the "sphinx" module itself (or any of its
submodules), right?

> I think
> providing API is not monkey-patching.

That's why I said "a disguised form of monkey-patching".

This was your example:

> ```
> # in nbsphinx/__init__.py
> custom_formats = {}
>
> def add_custom_format(suffix: str, reader: Callable) -> None:
> custom_formats[suffix] = reader
> ```

Without the provided API, the user could still do this:

```
import nbsphinx
nbsphinx.custom_formats[suffix] = reader
```

I would call this monkey-patching.

Your hypothetical function add_custom_format() probably makes this a
bit simpler, but it's mostly the same thing, isn't it?

And I think it's not good style (neither with nor without the helper function).

> As commented above, providing APIs are appropriate way to enhance the
> programs by code, I think.

Yes, definitely.

> And I consider the your idea; dictionary
> styled nbsphinx_custom_formats is a kind of code. It tries to
> represent code as data. It likes an idea of S-expressions.

Yes, I guess it is somewhere in the grey area between code and data.

But it still uses only Python literals.

AFAICT that's the main method to specify Sphinx configuration: by
assigning Python literals to some pre-defined variables.

I don't really like passing the dictionary, but it's the only
reasonable way I found to describe arbitrary keyword arguments in form
of Python literals.

> I agree my idea is not beautiful and simple. But I think it is
> difficult to do that simply because what you'd like to do is a
> programming, not a configuration.

Well, I would say it's somewhere in between.

From a user's point of view it shouldn't "feel" like writing code.

I guess users would mainly copy those lines from the documentation and
just change the relevant strings.

For a user, this might just look like JSON. I think the only
difference is that trailing commas are not allowed in JSON.

> > Also, I guess it wouldn't work with Sphinx's caching mechanism, would it?
> >
> > If a user would change the implementation of their setup() function,
> > Sphinx wouldn't re-build, right?
>
> Yes. It is hard to detect the change of the implementation.
>
> But is it available on your idea? If I set my custom function to
> `nbsphinx_custom_formats`, it is also hard to detect the change of the
> implementation.
>
> nbsphinx_custom_formats = {
> '.Rmd': ['mymodule.reads', {'fmt': '.Rmd'}],
> }

Sure, it wouldn't detect changes in the implementation of mymodule.reads().
But that's (typically) not implemented/changed by the user, so it
doesn't matter in this case.

But if some keyword arguments are changed, the caching mechanism will
easily detect that.
And that's the important part.

> Sphinx can only detects the change of configuration value, not the
> change of the target function.

Yes, that's totally fine for me.

That's one reason why I would prefer a configuration value over a
custom user-defined setup() function.

> > > I don't know this is easy to understand for users.
> >
> > I think having to create a setup() function is generally harder to
> > understand than assigning something to a configuration value.
> > I think "normal" users shouldn't worry at all about a setup() function.
>
> If my understanding correct, "normal" users you said are able to write
> python code, right?

Most users will know some programming language that's used in the
Jupyter ecosystem.
This doesn't have to be Python.

But probably the person who assembles the Jupyter notebooks didn't create them?
In this case they probably don't know Python.
And the shouldn't have to.

> Because I saw a lambda function in your first post (I don't know it is
> already implemented or not).
> If so, I don't think it is not difficult way to me.

The lambda function was just a work-around to be able to pass further
arguments to the conversion function.

I think it's nicer if no lambda function is needed.

> If not, I thought using configuration is also difficult for "normal"
> users. I think it is better to simplify it by lessening feature.

I agree, it's better to avoid the lambda function, which could be very
confusing for Python beginners.

> FWIW: recommonmark explains the way to enhance it via setup()
> function. I don't think this is a better way. But it's one of example
> for you.
> https://recommonmark.readthedocs.io/en/latest/#autostructify

Oh, that is ugly!

That's a horrible API!

A "normal" user should not have to *define* a config value.

And having to call app.add_transform() for "simple" usage is also bad.

This is an excellent example to show that a "normal" user should not
be required to define a setup() function.

It's hideous.

And it's exactly what I'm trying to avoid.

> > Do you have any ideas that don't involve the setup() function?
>
> Sorry, I have no idea more.

OK, no problem.
If something else comes to your mind, please let me know.

For now I have two options, both have their disadvantages, none of
them is really good:

1) define a setup() function

2) use a dict containing a list containing another dict

Both are kinda bad, but for me its clear that option (2) is still
significantly better.

It would be great is somebody could come up with an even better option, though!

cheers,
Matthias