8000 Reparametrize offsetbox calculations in terms of bboxes. by anntzer · Pull Request #24465 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]

Skip to content

Reparametrize offsetbox calculations in terms of bboxes. #24465

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
Dec 23, 2022

Conversation

anntzer
Copy link
Contributor
@anntzer anntzer commented Nov 15, 2022

Passing a single bbox instead of (xdescent, ydescent, width, height) separately is easier to follow (see e.g. the changes in VPacker and HPacker, which no longer have to repeatedly pack/unpack whd_list), and avoids having to figure out e.g. the sign of the descents and whether width/height includes the descents, for example.

Currently get_offset keeps a backcompatible signature (we could consider killing the old signature but let's not do that for now), and _get_bbox_and_child_offsets is private because I may want to later also change the convention to make offsets relative to the bbox (0, 0) point rather than the bbox lower-left corner.

PR Summary

PR Checklist

Tests and Styling

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • New plotting related features are documented with examples.

Release Notes

  • New features are marked with a .. versionadded:: directive in the docstring and documented in doc/users/next_whats_new/
  • API changes are marked with a .. versionchanged:: directive in the docstring and documented in doc/api/next_api_changes/
  • Release notes conform with instructions in next_whats_new/README.rst or next_api_changes/README.rst

Passing a single bbox instead of (xdescent, ydescent, width, height)
separately is easier to follow (see e.g. the changes in VPacker and
HPacker, which no longer have to repeatedly pack/unpack whd_list), and
avoids having to figure out e.g. the sign of the descents and whether
width/height includes the descents, for example.

Currently get_offset keeps a back compatible signature (we *could*
consider killing the old signature but let's not do that for now), and
_get_bbox_and_child_offsets is private because I *may* want to later
also change the convention to make offsets relative to the bbox (0, 0)
point rather than the bbox lower-left corner.

@functools.wraps(meth)
def get_offset(self, *args, **kwargs):
params = _api.select_matching_signature(sigs, self, *args, **kwargs)
Copy link
Member

Choose a reason for hiding this comment

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

Should probably warn if the old signature is used, select_matching_signature does not do so itself.

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 intentionally chose not to deprecate anything yet as I think we can live at least with one release supporting both apis to make sure the new one makes sense?

@ksunden ksunden merged commit b4474c4 into matplotlib:main Dec 23, 2022
@anntzer anntzer deleted the ob branch December 23, 2022 20:43
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.

4 participants
0