sphinx-thebe icon indicating copy to clipboard operation
sphinx-thebe copied to clipboard

Pass-through configuration directly to Thebe

Open choldgraf opened this issue 4 years ago • 7 comments

Description

For any configuration that exists in Thebe, we should simply pass it through directly to thebe rather than having our own configuration key name for it and translating it.

Benefit

This way we standardize on a single set of configuration keys, and can simply refer to the thebe docs for the full reference.

Implementation

Here is where thebe documents its configuration: https://thebe.readthedocs.io/en/latest/configure.html#

and we translate and configure thebe in this package here:

https://github.com/executablebooks/sphinx-thebe/blob/c036ac41a3b681ed4c2ab3b92edee4a3d2d2e1f0/sphinx_thebe/init.py#L124-L142

Note now we are "manually" translating many things into thebe's configuration. Instead of this, we should just accept something like thebe_config, which would be a dictionary that would directly map onto Thebe configuration.

choldgraf avatar Oct 22 '20 21:10 choldgraf

Coming here from #19, this seems like the preferred approach; is there anything blocking it?

akhmerov avatar Apr 29 '21 19:04 akhmerov

Nope, just hours in the day, I am happy to review a pr 🙂

choldgraf avatar Apr 29 '21 22:04 choldgraf

I can see what I can do. What's your preferred approach to backwards compatibility? A separate config variable + deprecation cycle? Moving fast and breaking things? Another option?

akhmerov avatar Apr 29 '21 22:04 akhmerov

Good question. Maybe we could add a key for "thebe_config_raw = True" or something, and deprecate over a cycle or two?

choldgraf avatar Apr 29 '21 22:04 choldgraf

TBH I'd also be fine just changing it now, as long as there wasn't a strong reason to keep the current behavior

choldgraf avatar Apr 29 '21 22:04 choldgraf

My preferred approach would be indeed to change the interface directly. The arguments why it wouldn't be too harmful are:

  • v0.0.x should be enough of a warning
  • the package users are likely capable of either pinning to the older version or updating to the new one

The only argument against I can see is the requirement of sphinx-thebe>=0.0.6 in jupyter-book.

akhmerov avatar Apr 29 '21 23:04 akhmerov

I good point, we should pin jupyter book to ~= 0..0.* first

choldgraf avatar Apr 30 '21 01:04 choldgraf