| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
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.
Sorry, something went wrong.
|
I'm always slightly annoyed when I have to write a version number but the format does not support dots. 08 is less readable than 0.8. 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 : or / may be more appropriate namespace separators. 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. |
Sorry, something went wrong.
|
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 mpl.style.use("foo.bar.baz") to import foo.bar; mpl.style.use(Path(foo.bar.__file__).parent / "baz.mplstyle")) where foo.bar is a package, so there's a foo/bar/__init__.py and a foo/bar/baz.mplstyle next to it (some important details are that we don't actually need to import foo.bar but can just locate it with importlib and therefore skip any execution of Python code (*), and the implementation also needs to be adjusted for namespace packages (i.e. foo/bar/__init__.py is not actually needed). We could say that this needs to be spelled e.g. mpl.style.use("foo.bar:baz") (somewhat similarly to pkgutil.resolve_name), but that may be more confusing for end users. Using slashes is likely worse because the front part is really a qualified package name (so that should really be dots), and "foo.bar/baz" seems a bit strange, plus you may confuse Windows users as to whether backslashes are needed or supported. (*) 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... |
Sorry, something went wrong.
|
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 'tab10:' and 'xkcd:' as prefixes to conflict with the named X11 colors. |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
|
That's fine with me too (and I don't reall care about legacy vs. original). Anyone else wants to pick the adjective? |
Sorry, something went wrong.
|
If nobody else has an opinion, I suggest to go with "legacy", but don't have a strong preference either. |
Sorry, something went wrong.
|
I actually prefer versions because what happens when seaborn changes again - seaborn-more-recent-legacy is going to get confusing. I'd just spell the version seaborn_v0_8. |
Sorry, something went wrong.
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. |
Sorry, something went wrong.
|
Discussed on the call, will change to seaborn-v0_8-XXXX and are on board with dropping . to preserve future flexibility as a special character. |
Sorry, something went wrong.
|
I may not be able to push a fix real quick, feel free to force push to rename that. |
Sorry, something went wrong.
|
dealing with this is on my plate for tonight. I assumed you would be ok with that @anntzer . |
Sorry, something went wrong.
|
Yes :) |
Sorry, something went wrong.
... 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.)
| Back | FazBrowse Home | New Git URL |
... 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.
(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
Documentation