[Bug]: rcdefaults and rcParams.clear() don't work?

[chahak13]

Bug summary

This is the rcdefaults() function

matplotlib/lib/matplotlib/__init__.py

Lines 1068 to 1091 in 8faa835

	def rcdefaults():
	"""
	Restore the `.rcParams` from Matplotlib's internal default style.
	Style-blacklisted `.rcParams` (defined in
	``matplotlib.style.core.STYLE_BLACKLIST``) are not updated.
	See Also
	--------
	matplotlib.rc_file_defaults
	Restore the `.rcParams` from the rc file originally loaded by
	Matplotlib.
	matplotlib.style.use
	Use a specific style file. Call ``style.use('default')`` to restore
	the default style.
	"""
	# Deprecation warnings were already handled when creating rcParamsDefault,
	# no need to reemit them here.
	with _api.suppress_matplotlib_deprecation_warning():
	from .style.core import STYLE_BLACKLIST
	rcParams.clear()
	rcParams.update({k: v for k, v in rcParamsDefault.items()
	if k not in STYLE_BLACKLIST})

What exactly are we trying to do here? According to the docstring, if we're trying to reset the rcParams dictionary and only reset the keys that are not in STYLE_BLACKLIST then there are 2 problems I think:

1. rcParams.clear() doesn't work. It doesn't clear out the entire dictionary.
2. After the update line, rcParams will still have keys from STYLE_BLACKLIST which, if I were to perform the same operations on a dictionary, shouldn't be the case.
3. While it does make sense to still have the keys from STYLE_BLACKLIST but then I'm not sure why clear() is used and not just directly update the keys.

Also, sidenote, popitem() errors out without a good error message (which is also why clear doesn't actually empty the dict?)

Code for reproduction

In [3]: mpl.rcParams["webagg.port"]
Out[3]: 8988

In [4]: mpl.rcParams["webagg.port"] = 9000

In [5]: mpl.rcParams["webagg.port"]
Out[5]: 9000

In [6]: mpl.rcdefaults()

In [7]: mpl.rcParams["webagg.port"]
Out[7]: 9000

In [8]: mpl.rcParams.clear()

In [9]: mpl.rcParams["webagg.port"]
Out[9]: 9000

Actual outcome

clear() doesn't empty the rcParams dictionary.

Expected outcome

clear() should empty the dictionary and after rcdefaults(), keys from STYLE_BLACKLIST shouldn't be there.

Additional information

I'm not sure if this is a documentation issue and the behaviour is intended or the behaviour isn't intended.

Operating system

No response

Matplotlib Version

Current main

Matplotlib Backend

No response

Python version

No response

Jupyter version

No response

Installation

None

[chahak13]

Seems like @timhoffm came across this while doing something similar in #12577 (comment)

The defaults should have also STYLE_BLACKLIST parameters.

The logic (iirc) was that anything that was not style should not be changed by the restore-default methods.

Based on @tacaswell's comment on that, it still doesn't make sense because right now they are persisted even though the code seems to be removing them?

I'm kinda confused about what exactly is the goal here.

[ksunden]

webagg.port is specifically in the STYLE_BLACKLIST which is explicitly excluded by the documentation for rcdefaults

The dict inheritance makes it confusing, as we don't actually directly implement __delitem__ but I am surprised that clear doesn't clear out everything.

If I call dict.clear(mpl.rcParams) it does clear out everything...

Somehow we are getting delitem and clear from MutableMapping and not actually properly inheriting from dict as was likely intended.

[ksunden]

For completeness, here is the STYLE_BLACKLIST:

{
    'webagg.port_retries',
    'figure.raise_window',
    'docstring.hardcopy',
    'backend',
    'backend_fallback',
    'toolbar',
    'figure.max_open_warning',
    'date.epoch',
    'timezone',
    'webagg.port',
    'webagg.open_in_browser',
    'webagg.address',
    'interactive',
    'tk.window_focus',
    'savefig.directory'
}

[chahak13]

Yep. I get that the STYLE_BLACKLIST is excluded but if clear() works as it normally would, then that means that webagg.port and any other entry in STYLE_BLACKLIST should not be in there.

[chahak13]

I think the reason it gets the MutableMapping is because our inheritance is defined as MutableMapping first and then dict.

[chahak13]

Basically, it feels like the "wrong" implementation is working right now?

[ksunden]

In general inheriting from dict was a mistake (and that is what #25617 is addressing)

If I do del mpl.rcParams["backend"] I get KeyError (despite the key actually being there)

Yeah, I think that rcdefaults is a no-op currently, which is not intended...

We do not test it at all.

Given that it broke and nobody complained... perhaps that is a sign it should be either deprecated completely or reworked entirely as part of #25617... It perhaps gives some latitude to make changes that might otherwise have warranted a difficult to navigate deprecation dance.

With the chainmap implementation (modulo the sub-dict structures) the documented behavior appears to be something like:

non_style = {k: self[k] for k in STYLE_BLACKLIST}
self.maps = [defaults, non_style]
self.maps.new_child()

(this is from the perspective of self being a ChainMap and defaults being available, but more or less that is I think what the documented behavior is)

And of course, should get a test...

[chahak13]

Yeah, I think I'll redo the implementation there to match the documentation more.