Simplify the rcparams deprecation machinery.

[anntzer]

Just have a single kind of deprecated rcParams, which emit a
DeprecationWarning on get and on set.

... which caught the fact that we were previously not warning on
access to text.latex.unicode, now we do but need to avoid triggering the
warnings internally.

PR Summary

PR Checklist

• Has Pytest style unit tests
• Code is Flake 8 compliant
• New features are documented, with examples if plot related
• Documentation is sphinx and numpydoc compliant
• Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
• Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

[timhoffm]

I'm 👎. Usage of dict.__getitem__(rcParams, par) for working around deprecation warnings is really a crude hack. I don't want to see this all over the code. Related: #12577.

[anntzer]

Right now read access on deprecated rcParams does not warn, which is quite clearly a bug. I think it is reasonable to say that rcParams["deprecated.key"] should warn both on read and on write. However additionally during the deprecation period we still need to be internally able to access the deprecated rcParams, so it's just a matter of how you want to spell that access -- dict.__getitem__(rcParams, "deprecated.key") currently works, so I think adding a rcParams._getitem_ignore_deprecation("deprecated.key") is not worth it, but I can do that, too... or we can also wrap each internal access with cbook._suppress_matplotlib_deprecation_warning() but again that seems slightly overly verbose (and would be slower).

[timhoffm]

I'd go with rcParams._getitem_ignore_deprecation("deprecated.key"). This at least communicates it's purpose clearly. A little less clear but also less verbose would be something like rcParams._data["deprecated.key"].

[anntzer]

Sure, added such a helper.

[timhoffm]

Actually I'm not sure if it's wise to throw away the parameter mapping capabilities. While we don't have any pararmeters in there right know, I can imagine that we may want to rename parameters in the future.

[anntzer]

We can always restore it if the need shows up, but right now it's quite a bit of complexity in the rcParams implementation for no use.

[timhoffm]

Hm, but fiddling that complexity back in is cumbersome as well and the present code doesn't hurt too much (or do you plan larger work on that part?). I'd llke to hear thoughts of the other devs on this.

[anntzer]

I don't plan to fiddle too much with it, but the complexity was such that the basic goal of this code (warning on access to deprecated rcParams, e.g. rcParams["text.latex.unicode"]) wasn't actually working... (Do the other cases work accurately? I don't know, and don't really want to write tests for them either.)

[tacaswell]

I am inclined to decline this PR.

[anntzer]

At least the problem that _deprecated_remains_as_none entries (e.g. text.latex.unicode -- now removed -- which is mentioned in the original post) did not trigger a warning on getitem remains.
In fact, now that I revisit this, I wonder if the right solution is not to completely get rid of deprecated{map,ignore_map,remain_as_none} and just write custom deprecation logic into setitem/getitem and add corresponding tests whenever we want to deprecate a rcparam...

[tacaswell]

... I wonder if the right solution is not to completely get rid of deprecated{map,ignore_map,remain_as_none} and just write custom deprecation logic into setitem/getitem and add corresponding tests whenever we want to deprecate a rcparam...

I think that this is a compelling position. We do it rarely enough and simply writing the code for it would likely be simpler and easier to read.

[github-actions]

Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it.

[anntzer]

@timhoffm Thoughts about just killing off the rc entire deprecation machinery (#15329 (comment)) and writing ad hoc code as needed?
If you're against it we should just close off this PR.

[timhoffm]

I support throwing out the whole machinery and write the code as needed.

[github-actions]

Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it.