8000 Factor out handling of missing spines in alignment calculations. by anntzer · Pull Request #28313 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]

Skip to content

Factor out handling of missing spines in alignment calculations. #28313

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
May 27, 2024

Conversation

anntzer
Copy link
Contributor
@anntzer anntzer commented May 27, 2024

... by using spines.get.

Note that for an Axes, get_window_extent (with or without the renderer argument) is always equal to ax.bbox; the refactoring also makes that symmetry clearer.

Noted while looking at #28300.

PR summary

PR checklist

@rcomer

This comment was marked as outdated.

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.

Take or leave my comments. You can self-merge.

@@ -2400,34 +2400,19 @@ def _update_label_position(self, renderer):
# get bounding boxes for this axis and any siblings
# that have been set by `fig.align_xlabels()`
bboxes, bboxes2 = self._get_tick_boxes_siblings(renderer=renderer)

ax = self.axes
Copy link
Member

Choose a reason for hiding this comment

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

This is only extracted to make l.2409 / l.2414 shorter. I would not do that because it separates definition and usage and thereby results in more state and thus cognitive load.

I slightly prefer to either leave this in place self.axes.spines.get("bottom", self.axes) - or if that's reads too cumbersome, define it right before usage in each if branch.

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 the repetition of self.axes reads a bit cumbersome here, but sure, I put it back. Ditto below.

@@ -2641,33 +2626,19 @@ def _update_label_position(self, renderer):
# get bounding boxes for this axis and any siblings
# that have been set by `fig.align_ylabels()`
bboxes, bboxes2 = self._get_tick_boxes_siblings(renderer=renderer)
ax = self.axes
Copy link
Member

Choose a reason for hiding this comment

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

As above.

... by using spines.get.

Note that for an Axes, get_window_extent (with or without the renderer
argument) is always equal to ax.bbox; the refactoring also makes that
symmetry clearer.
@timhoffm timhoffm added this to the v3.10.0 milestone May 27, 2024
@timhoffm timhoffm merged commit 2834da1 into matplotlib:main May 27, 2024
41 of 42 checks passed
@anntzer anntzer deleted the missing-spine branch May 27, 2024 18:26
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.

3 participants
0