[ENH] xkcd.mplstyle w/ quasi parsing of patheffects.{functions} #26854
Conversation
7e6b66b to
e4730c2
Compare
d2416cd to
6d35de0
Compare
6d78ec0 to
a5a7576
Compare
a5a7576 to
4e3f3cf
Compare
|
doc build failure mystifies me cause builds locally |
|
And it's an important doc build since it's the style example and unicode_minus keyword :/ |
|
Note to self:
|
|
A concern with using https://www.youtube.com/watch?v=iKzOBWOHGFE has a good section on inband vs out-of-band encoding of information. |
Technically with this format I can do Tuple[str, list, dict] for |
If you want But I suggest to keep it simple and only support kwargs. One could always add the other option later. |
|
It'd be really helpful if #26989 could get in b/c that would help me debug the unicode minus glyph not working error |
fc92806 to
45896e8
Compare
|
builds using https://github.com/dummy-index/xkcd-font/raw/brushup/xkcd-script/font/xkcd-script.ttf so do I need to verify that it's okay to use? liscence is cc and using this file b/c it has an extended character set |
|
Also, we may want to ask if we can vendor that file since it's on an unmerged branch of an unmaintained fork if we're gonna use it in doc builds? ETA: or fork that fork or the original project and add that fork? |
|
And do we wanna deprecate |
5e9e8a7 to
931cf4a
Compare
|
Can you please put the |
|
follow up from call is to make a note in xkcd docs about plt.style as alt |
adds tests and stricter validation for path.effects parameter Co-authored-by: Thomas A Caswell <tcaswell@gmail.com> Co-authored-by: Ben Root <ben.v.root@gmail.com> Co-authored-by: Antony Lee <anntzer.lee@gmail.com>
Co-authored-by: Antony Lee <anntzer.lee@gmail.com>
| # use patheffects object or instantiate patheffects.object(**kwargs) | ||
| return [pe if isinstance(pe, path_effects.AbstractPathEffect) | ||
| else getattr(path_effects, pe[0])(**pe[1]) | ||
| for pe in self._get('path.effects')] |
There was a problem hiding this comment.
This needs to support third party patheffects too (e.g. pkgname.modulename.patheffect instead of just withStroke)
|
notes: |
PR summary
Closes #5992 by parsing patheffects, is an alternative to #14943 and also builds on some of the ideas there and in #26050.
commit 2: swap xkcd script for humor sans[MNT] swap xkcd script for humor sans #27299@anntzer pointed out that input needs to be a list b/c the same patheffects function can be called multiple times and yield different results, so in this implementation
path.effectscan be set to:[patheffects.Normal(), patheffects.withStroke(linewidth=4)][('Normal', ), ('withStroke', {'linewidth':4})][patheffects.Normal(), ('withStroke', {'linewidth':4})]('Normal', ), ('withStroke', {'linewidth':4})I'm gonna defer writing a what's new and all the rest until there's some discussion/consensus on if this is a feasible approach. This PR introduces anxkcd.mplstyleand shims it intoxkcd.plotb/c that was the motivation for this PR, but I can also spin the xkcd stuff out into a separate PR/commit once decisions are more settled.For the arguments/keywords I went with a very generous "is this an ast_literal parsable dictionary" - advantage is it's a lot cleaner than the patheffects mini parser I started writing, disadvantage is that it's
not gonna error out til draw timeusing the function to do the validation.Definitely not the most optimal/optimized implementation, but this PR sketches out using a function name : dictionary approach, e.g:The advantage of this approach is that we can heavily restrict which patheffects to support and let rc_setup handle that validation, disadvantage is slighlty more typing.changed some of the path.sketch docs to clarify their format cause misreading them was what caused a lot of the rest of my test failures.spun out #26921 so that may need to go in firstPR checklist