Skip to content

Conversation

rcomer
Copy link
Member

@rcomer rcomer commented Aug 25, 2023

PR summary

Fixes #26588 following @anntzer's suggestion at #26588 (comment). The example code from #26588 (comment) now produces

image

Is there a better place to put the test? I wasn't expecting to create a new module...

PR checklist

@rcomer rcomer added this to the v3.7.3 milestone Aug 25, 2023
@rcomer rcomer added the PR: bugfix Pull requests that fix identified bugs label Aug 25, 2023
Comment on lines +7 to +10
def test_tick_labelcolor_array():
# Smoke test that we can instantiate a Tick with labelcolor as array.
ax = plt.axes()
XTick(ax, 0, labelcolor=np.array([1, 0, 0, 1]))
Copy link
Member

Choose a reason for hiding this comment

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

is there a test somewhere that this actually works?

Copy link
Member Author

Choose a reason for hiding this comment

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

I’m sorry I don’t understand the question.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, you've got this setup as a smoke test so I was wondering if somewhere in the code there's a test that passing an array to xtick color colors the tick the color of the array.

Copy link
Member

@timhoffm timhoffm Aug 27, 2023

Choose a reason for hiding this comment

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

One can add that check via tick.label1.get_color() (and also for label2; and if you want to be even more precise check that tick1line, tick2line and gridline do not get that color).

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, I did intend to respond to this but have not been well for a few days. It looks like I’m too late now but there is this test that sets a labelcolor with a string and then checks it via the getter:

https://github.com/matplotlib/matplotlib/blob/main/lib/matplotlib/tests/test_axes.py#L6778-L6788

Is that enough or is there a concern that using an array could break things further along the process than the instantiation?

Copy link
Member

Choose a reason for hiding this comment

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

Latter but also it's not a blocker and we can always add later if there's a bug that shows we should have added it ;)

Hope you're feeling better!

@ksunden ksunden merged commit e48bcaa into matplotlib:main Aug 28, 2023
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Aug 28, 2023
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Aug 28, 2023
ksunden added a commit that referenced this pull request Aug 28, 2023
…598-on-v3.7.x

Backport PR #26598 on branch v3.7.x (FIX: array labelcolor for Tick)
QuLogic added a commit that referenced this pull request Aug 28, 2023
…598-on-v3.8.x

Backport PR #26598 on branch v3.8.x (FIX: array labelcolor for Tick)
@rcomer rcomer deleted the tick-array-color branch August 29, 2023 18:04
@ksunden ksunden mentioned this pull request Sep 15, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: bugfix Pull requests that fix identified bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Tick class instantiation returns an error when labelcolor is a tuple
4 participants