-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Re-rename builtin seaborn styles to not include a dot. #23674
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! I'm not confident enough that there is a spurious failure, although I think so, but feel free to self-merge if you are.
I'm always slightly annoyed when I have to write a version number but the format does not support dots. Is it likely that we'll want the dot as a namespace separator later on and can't do with another good choice? For this the question is important, where we expect to use the name and whether that has restrictions. I think that Where might w want to use this?
Edit: OTOH the worst that is happening if we don't use a dot now is that we have less readable style names for the old seaborn styles. We can always add dots later and even add an alias for these seaborn styles. So feel free to merge if you think you want to reserve all liberties concerning dots. |
I do think that having not so nice names for old (somewhat discouraged) seaborn styles isn't a big deal (as noted in your edit). My (yet to be fully spelled out) proposal for pip-installable mplstyles is basically to map We could say that this needs to be spelled e.g. (*) As always, I don't agree that this is so important to avoid, but accept that that's probably the easiest way to get such a proposal moving forward... |
I lean toward taking this for now to make sure we do not paint our selves into a corner. Adding extra aliases (without removing the existing ones) is messy, but not disruptive. Other prior art we have is that for colors we use |
Yes at first I thought getting rid of the dots was strange, but agree that python library imports use the dots so if we want to import styles keeping dots out of the namespace makes sense. |
Thinking again: "seaborn08" is really ugly. Let's not do this. Instead, let's use a different name like "seaborn-legacy" or "seaborn-original". The suffix can basically be anything because we only need to make it clear it's not the same as (current) seaborn. We're not wedded to using a version number for this. If we introduce versioning of themes that will be a completely separate concept. |
That's fine with me too (and I don't reall care about legacy vs. original). Anyone else wants to pick the adjective? |
If nobody else has an opinion, I suggest to go with "legacy", but don't have a strong preference either. |
I actually prefer versions because what happens when seaborn changes again - |
That does not bother us. We don't and don't intend to ship current and future seaborn styles. I think this was discussed in a dev call but can't find the reference. The motivation for renaming is clarifying that the style we are shipping is not the current seaborn style. Any name except plain "seaborn" will do. Even if we should decide to ship seaborn or make it available as 3rd party installable package in the future that's not a problem. I very strongly advocate that if we open up the styling, we need a larger strategy for style handling, including versioning. Independent of that we'll have to support the existing names for backward compatibility. |
Discussed on the call, will change to |
I may not be able to push a fix real quick, feel free to force push to rename that. |
dealing with this is on my plate for tonight. I assumed you would be ok with that @anntzer . |
Yes :) |
... as we may want to use dots later to support third-party styles, e.g. `foo.bar` could be used to mean `foo/bar.mplstyle` where `foo` is a regularly installed python package.
… include a dot.
…674-on-v3.6.x Backport PR #23674 on branch v3.6.x (Re-rename builtin seaborn styles to not include a dot.)
... as we may want to use dots later to support third-party styles, e.g.
foo.bar
could be used to meanfoo/bar.mplstyle
wherefoo
is aregularly installed python package.
(This is an idea I had after rethinking about externally-distributable styles.)
Marking as RC as the original renaming also came in in mpl3.6, and we don't want to rename twice...
@tacaswell @timhoffm @QuLogic were involved in discussing the name in the original PR (#22317), so pinging them again here.
PR Summary
PR Checklist
Tests and Styling
pytest
passes).flake8-docstrings
and runflake8 --docstring-convention=all
).Documentation
doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).