Warning triggered for navigation_with_keys

[blink1073]

In many of the jupyter repos, we use pydata_sphinx_theme. All of our docs builds have started failing with 0.14.2 with the warning about default value of navigation_with_keys changing. Would it be possible to not trigger that warning? This seems like a lot of additional work for such a benign change. If this is going to be the policy of pydata_sphinx_theme in general, I'd like to know that as well so I can switch themes across the board.

https://github.com/jupyter/notebook/actions/runs/6634029880/job/18038165438

[12rambau]

full disclosure, the decision was taken here: #1503 (comment)

I would invite @drammock and @gabalafou to the thread as they worked on the linked PR.

I'm on @drammock side on this one, even though it's a small change, we inverted a parameter of our conf.py which will change the behaviour of all documentation relying on default values.

What we could do is raise it only if the value is not set, letting you with the easy navigation_with_keys: False to keep default behaviour without the warning.

[drammock]

What we could do is raise it only if the value is not set, letting you with the easy navigation_with_keys: False to keep default behaviour without the warning.

That is how it was supposed to work. Maybe that conf.py change will be enough to silence the warning; if not we should make it work that way and then release a hotfix

[blink1073]

In many cases we had no settings at all for the html theme. If there were a new single setting to opt out of these warnings I'd use that, otherwise this is too much of a maintenance cost for us.

[drammock]

I can confirm that setting

html_theme_options = {
    "navigation_with_keys": False,
}

in conf.py silences the warning, as intended. The warning is also silenced if you set it to True if you prefer the old default behavior. It only warns if the option is unset. After the next release, the default in theme.conf will change from unset (None) to False and the warning will be gone for good.

[drammock]

closing as the fix is entirely in user's conf.py, no changes to the theme needed. If I've missed something / the recommended fix doesn't work, please feel free to re-open.

[blink1073]

That doesn't fix my issue, we already knew that, but it confirms that we won't use this theme. Please reconsider the policy

[gabalafou]

This may be a total noob question, sorry if so, but why does Notebook use the -W option with sphinx-build? That's what is causing the doc build step in the CI to fail, correct?

I guess the thing that I don't understand here is that it seems that the theme needs a way to let users know about planned changes. But maybe it's an anti-pattern or unfriendly to CI systems to use a warning?

[blink1073]

We use -W to catch legitimate warnings from sphinx when something won't render correctly. Sphinx does not have this policy about warning about changing defaults from what I have seen. Unlike pytest, we don't have fine-grained control over which warnings to suppress. This is a theme, I trust the theme to have sensible defaults, and just update itself.

[drammock]

This is a theme, I trust the theme to have sensible defaults, and just update itself.

fair enough, and normally we don't warn about even very major changes like the recent accessibility-inspired color and style update. But this is a noticeable UX change for those who relied on it; we thought it was exceptional and so decided to warn.

If there were a new single setting to opt out of these warnings I'd use that, otherwise this is too much of a maintenance cost for us.

isn't the suggested change to conf.py providing just that? Or are you asking for some setting that will silence all warnings we might someday ever generate? We don't have such a mechanism in place.

We use -W to catch legitimate warnings from sphinx when something won't render correctly.

We do the same thing for our docs, and compare the captured warnings with a list of expected ones (there are some related to the "kitchen sink" page that we can't prevent because we don't control that page's content). You could adopt a similar strategy, and safe-list this particular warning.

[drammock]

I hear you that CIs breaking across multiple projects is a big headache and if you really want to jump ship to another theme obviously I can't stop you.

I'm reopening this issue because I'm willing to consider the perspective that warning here was inappropriate. We've had folks complain before about changes to private functions that we use in our __init__ so that influenced me to treat this more like a package (where we explictly now define a public API, and things like deprecation warnings are considered a normal good practice). I've not developed/maintained a theme prior to this one so if norms exist like "have sensible defaults and update yourself (without issuing warnings)" I am unaware of them.

Requesting @trallard @12rambau @choldgraf @jorisvandenbossche to weigh in here.

[vyasr]

I can confirm that setting

html_theme_options = {
    "navigation_with_keys": False,
}

in conf.py silences the warning, as intended. The warning is also silenced if you set it to True if you prefer the old default behavior. It only warns if the option is unset. After the next release, the default in theme.conf will change from unset (None) to False and the warning will be gone for good.

I'm afraid that this isn't the behavior we're currently observing. Here are a couple of PRs from the cudf repo where runs in the last 24 hours are showing the warning:

• First PR, along with the corresponding CI run and the output line showing the warning
• Second PR, along with the corresponding CI run and the output line showing the warning

This is happening despite the fact that we set navigation_with_keys: True explicitly in our conf.py. Are we missing something here?

[blink1073]

I think having an option like "suppress_warnings" would be enough to satisfy both sets of users. The sphinx "-w" option to write to a file and post-analyze isn't ergonomic and isn't something I want to do across all the jupyter repos.