GeezerVison

I don't understand why these are leading to broken tests? I think its obviously OK to break the tests for non-zero rotation, but breaking them for zero rotation doesn't seem right.

I don't understand why these are leading to broken tests? I think its obviously OK to break the tests for non-zero rotation, but breaking them for zero rotation doesn't seem right.

I think it is because when taking the components of the offset according to the non-rotation angle which is 0 degrees, we are setting y0 to be sin(0)*old y0 = 0. But I don’t think there should be a y offset due to y0, there is a y offset regardless due to yd. So that is why it might break tests with non-rotated text.

I think you'll need to give us a little more detail. We can't break everyone who depends on our code without a really good reason. I think this would be easier with a diagram and some numbers. I appreciate that none of this is helped by the fact that draw_text_image is not documented, so its hard to know what the right thing to do is. But we should be certain we get this right if we are changing it.

This fix and #19114 is different because #19114 tried to fix the bounding box and we are trying to change the offset for the text. But the problem is not with the bounding box but rather the text itself - here's an example to prove it:

import matplotlib.pyplot as plt

fig, axes = plt.subplots(2, 2)
for idx, ax in enumerate(axes.flatten()):
      ax.axvline(.5, linewidth=.5, color='.5')
      ax.axhline(.5, linewidth=.5, color='.5')
      ax.text(
          .5, .5, ' ', size=50,
          bbox=dict(facecolor='None', edgecolor='red', pad=0),
          rotation=idx*90,
          va='center_baseline',
          rotation_mode='anchor'
      )

plt.show()

Here's an example of text without the bounding box:

import matplotlib.pyplot as plt

fig, axes = plt.subplots(2, 2)
for idx, ax in enumerate(axes.flatten()):
      ax.axvline(.5, linewidth=.5, color='.5')
      ax.axhline(.5, linewidth=.5, color='.5')
      ax.text(
          .5, .5, 'pP', size=50,
          #bbox=dict(facecolor='None', edgecolor='red', pad=0),
          rotation=idx*90,
          va='center_baseline',
          rotation_mode='anchor'
      )

plt.show()

And you can see there is a problem with the offset of the text even when a bounding box isn't drawn.

I think you'll need to give us a little more detail. We can't break everyone who depends on our code without a really good reason. I think this would be easier with a diagram and some numbers. I appreciate that none of this is helped by the fact that draw_text_image is not documented, so its hard to know what the right thing to do is. But we should be certain we get this right if we are changing it.

So I never could find the draw_text_image function in the source code actually. I was trying to look for it to check whether it solves rotation correctly or not.

Yes, draw_text_image is implemented in the agg c-code. (_backend_agg_wrapper.cpp).

Closing and reopening to get fresh test results.

👋🏻 Hello, @safdarzareef ! Are you interested in picking this back up? Can we help you with this? Please let us know 😄

Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it.