Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

get_config fails with any section that is not 'theme' or 'options' #12305

Closed
Carreau opened this issue Apr 18, 2024 · 4 comments
Closed

get_config fails with any section that is not 'theme' or 'options' #12305

Carreau opened this issue Apr 18, 2024 · 4 comments
Labels

Comments

@Carreau
Copy link

Carreau commented Apr 18, 2024

Describe the bug

See sunpy/ablog#277

in 360c7a8 the following change was introduced:

-        try:
-            return self.config.get(section, name)
-        except (configparser.NoOptionError, configparser.NoSectionError):
-            if self._base:
-                return self._base.get_config(section, name, default)
-
-            if default is _NO_DEFAULT:
-                raise ThemeError(__('setting %s.%s occurs in none of the '
-                                    'searched theme configs') % (section, name)) from None
-            return default
+        if section == 'theme':
+            value = self._settings.get(name, default)
+        elif section == 'options':
+            value = self._options.get(name, default)
+        else:
+            value = _NO_DEFAULT
+        if value is _NO_DEFAULT:
+            msg = __(
+                'setting %s.%s occurs in none of the searched theme configs',
+            ) % (section, name)
+            raise ThemeError(msg)
+        return value

This means that any section name that is not 'theme', or 'options' will raise. Extensions, (typically ablog) use section names that are not 'theme' or 'options', and call get_config with a default values, failing.

https://github.com/sunpy/ablog/blob/b39f90bda1fc2bfddc373f5e5e970201914e1549/src/ablog/__init__.py#L121C14-L121C21

How to Reproduce

Build anything that use ablog as extension with sphin
Build pydata-sphinx-theme docs by default for example

Environment Information

sphinx 7.3.6

Sphinx extensions

ablog

Additional context

No response

@AA-Turner
Copy link
Member

Thanks for reporting.

The rationale for the changes is to tighten the theme configuration interface to allow supporting theme.toml. The documentation has never advertised support for other sections, and whilst it has not been prohibited, it is not supported.

We could make the below change to remove the error, but this would mask the true behaviour, as Sphinx 7.3 and newer do have a behavioural change in that these other sections are unsupported.

 else: 
-    value = _NO_DEFAULT
+    value = default

sphinx/sphinx/theming.py

Lines 105 to 130 in 630b4fb

def get_config(self, section: str, name: str, default: Any = _NO_DEFAULT) -> Any:
"""Return the value for a theme configuration setting, searching the
base theme chain.
"""
if section == 'theme':
if name == 'stylesheet':
value = ', '.join(self.stylesheets) or default
elif name == 'sidebars':
value = ', '.join(self.sidebar_templates) or default
elif name == 'pygments_style':
value = self.pygments_style_default or default
elif name == 'pygments_dark_style':
value = self.pygments_style_dark or default
else:
value = default
elif section == 'options':
value = self._options.get(name, default)
else:
value = _NO_DEFAULT
if value is _NO_DEFAULT:
msg = __('setting %s.%s occurs in none of the searched theme configs') % (
section,
name,
)
raise ThemeError(msg)
return value

A

@AA-Turner
Copy link
Member

4a0c9dd

@AA-Turner
Copy link
Member

Sphinx 7.3.7 has been released with fixes.

A

@Carreau
Copy link
Author

Carreau commented Apr 19, 2024

Thanks for the quick fix !

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants