Skip to content
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

Add style for xkcd #6157

Closed
wants to merge 1 commit into from
Closed

Add style for xkcd #6157

wants to merge 1 commit into from

Conversation

souravsingh
Copy link
Contributor

Added a style file for xkcd
Fixes #5992

Added a style file for xkcd
Fixes #5992

##PATH

path.sketch : (scale, length, randomness)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't these be numbers, rather than the placeholders "scale, length, randomness"?

@mdboom
Copy link
Member

mdboom commented Mar 14, 2016

This is missing the patheffect that puts a white border behind all lines.

@tacaswell tacaswell added this to the 2.1 (next point release) milestone Mar 16, 2016
@afvincent afvincent self-assigned this Jul 18, 2016
@afvincent
Copy link
Contributor

Ping @souravsingh : are you still interested working on this style sheet? With @mdboom's remarks, I guess one should get something that starts to resemble xkcd plots :).

@souravsingh
Copy link
Contributor Author

@afvincent I am interested in working on the style sheet. I am confused about the parameter for patheffect

@WeatherGod
Copy link
Member

The patheffect part is probably going to be the hardest part at the moment,
since it doesn't exist as a style parameter yet.

On Mon, Jul 18, 2016 at 9:22 AM, Sourav Singh notifications@github.com
wrote:

@afvincent https://github.com/afvincent I am interested in working on
the style sheet. I am confused about the parameter for patheffect


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#6157 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AARy-Kj819aprk-HzdGTI4qYEXge__T_ks5qW34ogaJpZM4HwCQT
.

@afvincent
Copy link
Contributor

@WeatherGod You're right: the patheffect part might be a bit more difficult to setup than I expected :/. I was fooled by the example of rcParams['path.effects'] = [patheffects.withStroke(linewidth=4, foreground="w")] where the relevant rcParams key exists. But patheffects is actually loaded elsewhere before…

@afvincent
Copy link
Contributor

@souravsingh Apparently, there is an official xkcd font, so it could be nice to add it as the default choice (another PR is open to add it to plt.xkcd, see #6811 for details). I don't have a solution for the patheffect part yet, but I'll try to have a look at this problem during the weekend.

@souravsingh
Copy link
Contributor Author

@afvincent Is the problem fixed?

@afvincent
Copy link
Contributor

Wohh I totally forgot this one : apologies @souravsingh! The xkcd font seems to have been added to the list so this part should be OK.

Unfortunately, I cannot see an easy solution to the patheffect part 😞. The validator of the corresponding rcparam (rcParams['path.effects']) would require to eval a string like "patheffects.withStroke(linewidth=4, foreground='w')", which may raise security issues if I am correct. But maybe a core dev. will prove me wrong ^^.

@dopplershift
Copy link
Contributor

@afvincent Well, we control that string, and patheffects, so it's certainly possible to create some kind of string syntax that we can then parse ourselves without using eval....

@tacaswell tacaswell modified the milestones: 2.1 (next point release), 2.2 (next next feature release) Aug 29, 2017
@anntzer anntzer mentioned this pull request Oct 22, 2017
1 task
@jklymak
Copy link
Member

jklymak commented Oct 22, 2017

If we parsed path.effect as a string like "withStroke(linewidth=4, foreground='w')" and then did something like:

patheffect = getattr(patheffects.modules[__name__], str)
rc['path.effect'] = patheffect(**kwargs)

would that be safe enough? ping @anntzer

@anntzer
Copy link
Contributor

anntzer commented Oct 22, 2017

Please no more parsing craziness.

@souravsingh
Copy link
Contributor Author

Is there a resolution for the PR?

@anntzer
Copy link
Contributor

anntzer commented May 13, 2018

I hope the resolution will be #9528.

@anntzer anntzer mentioned this pull request Jul 31, 2019
6 tasks
@jklymak
Copy link
Member

jklymak commented Jul 14, 2020

I'll close in favour of #14943

@jklymak jklymak closed this Jul 14, 2020
@story645 story645 removed this from the future releases milestone Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add xkcd style
9 participants