Skip to content

Modify rainbow_text() function to use annotate() function #25993

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 8 commits into from
Jun 3, 2023

Conversation

niranjank2022
Copy link
Contributor

@niranjank2022 niranjank2022 commented May 28, 2023

PR summary

This pull request is to modify the rainbow_text() function to use annotate() method. Closes #25941

PR checklist

@niranjank2022
Copy link
Contributor Author

Please tell whether my code is correct or anything can be changed. After that, I will work on documentation.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a while, please feel free to ping @matplotlib/developers or anyone who has commented on the PR. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join us on gitter for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide

We strive to be a welcoming and open project. Please follow our Code of Conduct.


def rainbow_text(x, y, strings, colors, orientation='horizontal',
ax=None, **kwargs):
"""
Take a list of *strings* and *colors* and place them next to each
Copy link
Member

Choose a reason for hiding this comment

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

Please leave the docstring in place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.

t = text.get_transform() + \
offset_copy(Affine2D(), fig=fig, x=0, y=ex.height)
txt = ax.text(x, y, strings[0], color=colors[0], **kwargs)
for s, c in zip(strings[1:], colors[1:]):
Copy link
Member

Choose a reason for hiding this comment

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

you can directly do txt = annotate() even if you use the previous value of txt as parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I have done it now.

@anntzer
Copy link
Contributor

anntzer commented May 28, 2023

The idea is correct. As noted by @timhoffm the code/docs simply needs to be cleaned up now.

Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

This introduces larger (unnatural) spacing.

new:
grafik

old:
grafik

@anntzer maybe you can comment on this as the use of annotate() is your proposal.

@anntzer
Copy link
Contributor

anntzer commented Jun 1, 2023

Looks like just adding the space explicitly to each word (as was done previously) and not adding any offset works:

    if orientation == 'horizontal':
        txt = ax.text(x, y, strings[0] + " ", color=colors[0], **kwargs)
        for s, c in zip(strings[1:], colors[1:]):
            txt = ax.annotate(s + " ", xy=(1, 0), xycoords=txt,
                              va="bottom", color=c, **kwargs)

    elif orientation == 'vertical':
        kwargs.update(rotation=90, verticalalignment='bottom')
        txt = ax.text(x, y, strings[0] + " ", color=colors[0], **kwargs)
        for s, c in zip(strings[1:], colors[1:]):
            txt = ax.annotate(s + " ", xy=(0, 1), xycoords=txt,
                              va="bottom", color=c, **kwargs)

@niranjank2022
Copy link
Contributor Author

Looks like just adding the space explicitly to each word (as was done previously) and not adding any offset works:

    if orientation == 'horizontal':
        txt = ax.text(x, y, strings[0] + " ", color=colors[0], **kwargs)
        for s, c in zip(strings[1:], colors[1:]):
            txt = ax.annotate(s + " ", xy=(1, 0), xycoords=txt,
                              va="bottom", color=c, **kwargs)

    elif orientation == 'vertical':
        kwargs.update(rotation=90, verticalalignment='bottom')
        txt = ax.text(x, y, strings[0] + " ", color=colors[0], **kwargs)
        for s, c in zip(strings[1:], colors[1:]):
            txt = ax.annotate(s + " ", xy=(0, 1), xycoords=txt,
                              va="bottom", color=c, **kwargs)

Is there any link to know more about the available arguments in annotate()?

@oscargus
Copy link
Member

oscargus commented Jun 2, 2023

Is there any link to know more about the available arguments in annotate()?

https://matplotlib.org/stable/api/_as_gen/matplotlib.axes.Axes.annotate.html#matplotlib.axes.Axes.annotate

@niranjank2022
Copy link
Contributor Author

Is there any link to know more about the available arguments in annotate()?

https://matplotlib.org/stable/api/_as_gen/matplotlib.axes.Axes.annotate.html#matplotlib.axes.Axes.annotate

And also about the **kwargs arguments?

@timhoffm
Copy link
Member

timhoffm commented Jun 2, 2023

image

@story645
Copy link
Member

story645 commented Jun 2, 2023

Screenshot_20230602-100742.png

At the bottom of this (and should be any) parameters list, there's a link out to where the **kwargs are explained. Here the **kwargs are anything that can be passed into a Text object constructor.

@niranjank2022
Copy link
Contributor Author

Screenshot_20230602-100742.png

At the bottom of this (and should be any) parameters list, there's a link out to where the **kwargs are explained. Here the **kwargs are anything that can be passed into a Text object constructor.

Thanks a lot.

Copy link
Contributor

@anntzer anntzer left a comment

Choose a reason for hiding this comment

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

Looks good for now, let's move further cleanup in another PR.

@anntzer anntzer dismissed timhoffm’s stale review June 3, 2023 10:25

Spacing fixed. Thanks for pointing out the issue (leading in particular to #26028).

@anntzer anntzer merged commit 5f25d20 into matplotlib:main Jun 3, 2023
@oscargus oscargus added this to the v3.8.0 milestone Jun 4, 2023
melissawm pushed a commit to melissawm/matplotlib that referenced this pull request Jun 15, 2023
…#25993)

* Modify rainbow_text() function to use annotate() function

* Add doctring

* Adjust code

* Add doctring for function

* Remove the variable t

* Modify function doctring

* Modify function doctring

* Remove the excess space
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

[Doc]: Rewrite rainbow_text example to use annotate()
6 participants