Description
What is your suggestion?
The following related PRs have been merged since v5.4.1
release:
This issue is the continuation of the discussion below:
Originally posted by @dangotbanned in post review comment
...
In #3536 (comment) I mentioned some related issues, but can now see I forgot to elaborate on what I would prefer for v6
.
I would like to have used altair.theme
as the namespace instead of altair.typing.theme
.
However, this would be very easy to mistake for altair.themes
- which is a variable storing a registry of themes.
For v6
I would propose the following api changes - and if there is any support I will open a new issue:
altair.typing.theme
->altair.theme
- Including dropping
altair.typing.ThemeConfig
, which is already included in the above
- Including dropping
@altair.register_theme
->@altair.theme.register
- New function:
altair.theme.enable
- Remove
altair.themes
top-level export
This would consolidate all theme related functionality behind a single namespace:
from altair import theme
@theme.register("custom 1", enable=False)
def custom_theme() -> theme.ThemeConfig:
return {
"autosize": {"contains": "content", "resize": True},
"config": theme.ConfigKwds(
circle=theme.MarkConfigKwds(fill="purple")
),
}
...
theme.enable("custom 1")
...
theme.enable("default")
Response by @mattijn in comment
...
I like the suggestion you provide in #3536 (comment) to consolidate everything under a single namespace, including the proposed ThemeConfig
.
I’m fine with doing that now, rather than releasing a version first and requiring deprecation immediately after.
However, instead of phasing out the plural altair.themes
in favour of the singleton altair.theme
can we consolidate this PR's work into the existing altair.themes
?
For example, can we have altair.themes.ThemeConfig
(and altair.themes.AxisConfigKwds
, etc.) aligning with the existing altair.themes.register()
and altair.themes.enable()
.
Aesthetically I like the singleton altair.theme
, but I think sticking with altair.themes
is more practical, if that is also an option. Can you reflect on this?
Also tagging @binste, as our discussion in another issue probably needs consideration
Notes
alt.vegalite.v5.theme
,alt.utils.theme
were not mentioned in the discussion - stating them here for completeness
Edit
@mattijn apologies for the delay, but see #3610 (comment) for my thoughts