Skip to content

Fix default parameters of FancyArrow #6583

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

Merged
merged 1 commit into from
Oct 3, 2016
Merged

Conversation

alcinos
Copy link
Contributor

@alcinos alcinos commented Jun 14, 2016

Make code consistent with documentation for FancyArrow

Make code consistent with documentation for FancyArrow
@tacaswell
Copy link
Member

Probably should make the documentation consistent with the code?

@alcinos
Copy link
Contributor Author

alcinos commented Jun 15, 2016

Not sure about this, because the default behave somewhat weirdly…
Here is the default looking arrow:
default
Notice that the width is really small.

If we set the width to something more sensible (in this case width=0.05):
default_wide
which doesn't make sense at all.

With the original defaults stated in the documentation it looks like this:
better

Perhaps the linear dependance is really the problem here.

@tacaswell tacaswell added this to the 2.0 (style change major release) milestone Jul 2, 2016
@tacaswell
Copy link
Member

What does the default arrow now look like?

I am mostly 👎 on this change, but am open to be convinced otherwise.

The history of this seems to be, the default was changed to 20 (but the docs not updated) in 2013 in (7458091 / #1186) because the head was invisible.

The default width of 0.001 seems to go all the way back to when this code came into the code base de93f2b

I think this either needs to also change the default width, or just update the docs to match the code.

import matplotlib.patches as mpatches

fig, ax = plt.subplots()

scales = [3, 20]
widths = [0.001, .01, .05]

for k, scale in enumerate(scales):
    ax.axvline(k, color='k', ls=':')
    ax.axvline(k+.5, color='k', ls=':')
    for j, width in enumerate(widths):
        a = mpatches.FancyArrow(k, j, .5, 0, width=width, head_width=scale*width)
        ax.add_patch(a)

ax.set_ylim(-1, j+1)
ax.set_xlim(-.25, k + 1)

ax.set_xticks(np.arange(len(scales)) + .25)
ax.set_xticklabels(map(str, scales))
ax.set_yticks(range(len(widths)))
ax.set_yticklabels(map(str, widths))

ax.set_xlabel('default scale')
ax.set_ylabel('default width')

so

attn @dmcdougall @pelson

@efiring
Copy link
Member

efiring commented Jul 2, 2016

Part of the real problem is that Arrow and FancyArrow make sense (if at all) only if drawn with a transform that makes the x and y scales identical. Hence they are of very limited utility. I think both should be deprecated--unless I am missing some application for which they are well-suited. Nevertheless, the present default of having the head width be 20 times the tail width seems a bit extreme.

@QuLogic
Copy link
Member

QuLogic commented Jul 10, 2016

What are the units for the width? It's clearly not pixels, but maybe it's time to increase the default, since it seems to be rounded up to one pixel anyway...

@tacaswell
Copy link
Member

After discussion with @efiring and @dopplershift we decided to merge this as-is and then deprecate this Artist.

@tacaswell tacaswell merged commit 9205967 into matplotlib:master Oct 3, 2016
QuLogic pushed a commit to QuLogic/matplotlib that referenced this pull request Oct 16, 2016
API: default parameters of FancyArrow
@QuLogic
Copy link
Member

QuLogic commented Oct 18, 2016

Backported to v2.x via 848cfe5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants