Skip to content

19195 rotated markers #20914

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 22 commits into from
Oct 5, 2021
Merged

Conversation

deep-jkl
Copy link
Contributor

@deep-jkl deep-jkl commented Aug 26, 2021

PR Summary

Add user supplied transforms and join/cap styles …

Improvement was done in instantiating new instance utilizing deep copy should preserve immutability of MarkerStyle members (e.g. Path, or Transform). However deepcopy of MarkerStyle(r"$|||$") fails. Originally it was needed in lines.py, but I think it can be optimized out to avoid multiple re-instantiation of MarkerStyle.

I followed the discussion for issue #19195 and tried several cases:

from matplotlib import transforms
import matplotlib.pyplot as plt
import matplotlib.pyplot as plt
from matplotlib.lines import Line2D
from matplotlib.transforms import Affine2D
from matplotlib.markers import MarkerStyle, JoinStyle, CapStyle

text_style = dict(horizontalalignment='right', verticalalignment='center',
                  fontsize=12, fontfamily='monospace')
marker_style = dict(linestyle=':', color='0.8', markersize=10,
                    markerfacecolor="tab:blue", markeredgecolor="tab:blue")

def format_axes(ax):
    ax.margins(0.2)
    ax.set_axis_off()
    ax.invert_yaxis()

fig, ax = plt.subplots()
fig.suptitle('Marker fillstyle', fontsize=14)
fig.subplots_adjust(left=0.4)

marker_colors = dict(markersize=15,
                    markerfacecolor='tab:blue',
                    markerfacecoloralt='lightsteelblue',
                    markeredgecolor='brown')

for y, fill_style in enumerate(Line2D.fillStyles[1:-1]):
    ax.text(-0.5, y, repr(fill_style), **text_style)
    ax.plot([0,5],[y]*2, linestyle=":", color='darkgrey')
    for x, theta in enumerate([0, 10, 15, 20, 30, 45]):
        ax.plot(x, y, marker=MarkerStyle("o", fill_style, transform=Affine2D().rotate_deg(theta)), **marker_colors)
        ax.text(x, 4, f"{theta}°", **text_style)
format_axes(ax)

plt.show()

fillstyle

fig, ax = plt.subplots()
fig.suptitle('Marker CapStyle', fontsize=14)
fig.subplots_adjust(left=0.1)

marker_inner = dict(markersize=35,
                    markerfacecolor='tab:blue',
                    markerfacecoloralt='lightsteelblue',
                    markeredgecolor='brown',
                    markeredgewidth=8,
                    )

marker_outer = dict(markersize=35,
                    markerfacecolor='tab:blue',
                    markerfacecoloralt='lightsteelblue',
                    markeredgecolor='white',
                    markeredgewidth=1,
                    )

for y, cap_style in enumerate(CapStyle):
    ax.text(-0.5, y, cap_style.name, **text_style)
    ax.plot([0,5],[y]*2, linestyle=":", color='darkgrey')
    for x, theta in enumerate([0, 10, 15, 20, 30, 45]):
        ax.plot(x, y, marker=MarkerStyle("1", transform=Affine2D().rotate_deg(theta), capstyle=cap_style), **marker_inner)
        ax.plot(x, y, marker=MarkerStyle("1", transform=Affine2D().rotate_deg(theta), capstyle=cap_style), **marker_outer)
        ax.text(x, 3, f"{theta}°", **text_style)
format_axes(ax)

plt.show()

capstyle

fig, ax = plt.subplots()
fig.suptitle('Marker JoinStyle', fontsize=14)
fig.subplots_adjust(left=0.1)

marker_colors = dict(markersize=35,
                    markerfacecolor='tab:blue',
                    markerfacecoloralt='lightsteelblue',
                    markeredgecolor='brown',
                    markeredgewidth=8)

for y, join_style in enumerate(JoinStyle):
    ax.text(-0.5, y, join_style.name, **text_style)
    ax.plot([0,5],[y]*2, linestyle=":", color='darkgrey')
    for x, theta in enumerate([0, 10, 15, 20, 30, 45]):
        ax.plot(x, y, marker=MarkerStyle("*", transform=Affine2D().rotate_deg(theta), joinstyle=join_style), **marker_colors)
        ax.text(x, 3, f"{theta}°", **text_style)
format_axes(ax)

plt.show()

joinstyle

list1 = [0, 1] #| x coordinates
list2 = [0, 0] #| y coordinates

p1 = [.5, 0] #| p1 stands for point1, the marker will be placed in this point
marker1 = MarkerStyle(r'$|||$', transform=Affine2D().rotate_deg(45))

plt.plot(list1, list2, 'o-k', linewidth = 1)
plt.plot(p1[0], p1[1], 'k', marker = marker1)

#T# show the results
plt.show()

reproductor

Things that needs to be done:

  • Add modifiers such as (rotated -> returns new rotated instance, and other primitive transformations)
  • Add tests
  • Improve documentation

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • [ N/A ] API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

@deep-jkl deep-jkl marked this pull request as ready for review September 13, 2021 08:30
Copy link
Member

@dstansby dstansby left a comment

Choose a reason for hiding this comment

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

I think this looks 👍 overall. I've left some comments and questions.

@jklymak
Copy link
Member

jklymak commented Sep 21, 2021

@deep-jkl this has developed a merge conflict. I don't think that should affect his reviewable it is, but will need to be fixed. It looks like some others have put some time into this, so feel free to ping them again if it is ready for a new look. Thanks for your patience!

@deep-jkl deep-jkl force-pushed the 19195-rotated-markers branch from 1aef5fa to 75a92d8 Compare September 21, 2021 19:37
@deep-jkl
Copy link
Contributor Author

@deep-jkl this has developed a merge conflict. I don't think that should affect his reviewable it is, but will need to be fixed. It looks like some others have put some time into this, so feel free to ping them again if it is ready for a new look. Thanks for your patience!

Thanks for the reminder, I hoped that you were just busy with the release.

@timhoffm @dstansby I have resolved conflict and comments, can you have a look on this PR?

@timhoffm
Copy link
Member

@deep-jkl thanks for following up. It'll be a couple of days before I can have a look.

Copy link
Member

@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

This is looking good to me. I made a bunch of suggestions, mostly to do w/ the doc strings...

deep-jkl and others added 3 commits September 28, 2021 09:26
Apply suggestions from @jklymak code review

Co-authored-by: Jody Klymak <jklymak@gmail.com>
Apply suggestions from code review

Co-authored-by: Jody Klymak <jklymak@gmail.com>
@deep-jkl
Copy link
Contributor Author

This is looking good to me. I made a bunch of suggestions, mostly to do w/ the doc strings...

Thanks for review, I have processed all the suggestions so far.

@jklymak jklymak added this to the v3.6.0 milestone Sep 28, 2021
Check for user style in get_cap(join)style is removed.
This is also supported with two new tests.
Additional test checks that get_transform returns combination of
supplied and internal transformation.
Copy link
Member

@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

This looks good to me, and is an oft-requested feature that I think will make a lot of people happy!

@dstansby dstansby merged commit fcfea77 into matplotlib:master Oct 5, 2021
tacaswell pushed a commit to tacaswell/matplotlib that referenced this pull request Oct 12, 2021
tacaswell pushed a commit that referenced this pull request Oct 20, 2021
@deep-jkl deep-jkl deleted the 19195-rotated-markers branch April 13, 2022 18:31
QuLogic added a commit to QuLogic/matplotlib that referenced this pull request May 6, 2023
When these Enum classes were added in matplotlib#18544, they were supposed to be
for documentation only. To that end, matplotlib#22055 was a followup that ensured
that only the strings were exposed from the getter side.

However, when user-supplied cap/join style were added in matplotlib#20914, they
were only for the Enum type instead of the string, so correctly allow
strings here as well.

Also, specifically type hint the return values as literals, as was done
in matplotlib#25719.
QuLogic added a commit to QuLogic/matplotlib that referenced this pull request May 6, 2023
When these Enum classes were added in matplotlib#18544, they were supposed to be
for documentation only. To that end, matplotlib#22055 was a followup that ensured
that only the strings were exposed from the getter side.

However, when user-supplied cap/join style were added in matplotlib#20914, they
were only for the Enum type instead of the string, so correctly allow
strings here as well.

Also, specifically type hint the return values as literals, as was done
in matplotlib#25719.
@QuLogic QuLogic mentioned this pull request May 6, 2023
1 task
QuLogic added a commit to QuLogic/matplotlib that referenced this pull request May 31, 2023
When these Enum classes were added in matplotlib#18544, they were supposed to be
for documentation only. To that end, matplotlib#22055 was a followup that ensured
that only the strings were exposed from the getter side.

However, when user-supplied cap/join style were added in matplotlib#20914, they
were only for the Enum type instead of the string, so correctly allow
strings here as well.

Also, specifically type hint the return values as literals, as was done
in matplotlib#25719.
devRD pushed a commit to devRD/matplotlib that referenced this pull request Jun 5, 2023
When these Enum classes were added in matplotlib#18544, they were supposed to be
for documentation only. To that end, matplotlib#22055 was a followup that ensured
that only the strings were exposed from the getter side.

However, when user-supplied cap/join style were added in matplotlib#20914, they
were only for the Enum type instead of the string, so correctly allow
strings here as well.

Also, specifically type hint the return values as literals, as was done
in matplotlib#25719.
melissawm pushed a commit to melissawm/matplotlib that referenced this pull request Jun 15, 2023
When these Enum classes were added in matplotlib#18544, they were supposed to be
for documentation only. To that end, matplotlib#22055 was a followup that ensured
that only the strings were exposed from the getter side.

However, when user-supplied cap/join style were added in matplotlib#20914, they
were only for the Enum type instead of the string, so correctly allow
strings here as well.

Also, specifically type hint the return values as literals, as was done
in matplotlib#25719.
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.

5 participants