Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Fix axis3d to include offset text in tight bounding box calculation#30760

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
FazeelUsmani wants to merge6 commits intomatplotlib:main
base:main
Choose a base branch
Loading
fromFazeelUsmani:fix-axis3d-offset-text-tightbbox

Conversation

@FazeelUsmani
Copy link
Contributor

#30744

The fix follows the same pattern used in the 2D axis implementation (lib/matplotlib/axis.py:1366-1369), which includesoffsetText in the bounding box calculation by:

  1. Checking ifoffsetText is visible
  2. Getting its window extent (bounding box)
  3. Including it in the union of bounding boxes

The implementation adds these two lines toaxis3d.py:

ifself.offsetText.get_visible():other.append(self.offsetText.get_window_extent(renderer))

This ensures consistency between 2D and 3D axis behavior.

Minimum self-contained example

importnumpyasnpimportmatplotlib.pyplotasplt# Create data with high precision values that trigger offset textZ=np.array([[0.1,0.100000001], [0.100000000001,0.100000000]])ny,nx=Z.shapex=np.arange(nx)y=np.arange(ny)X,Y=np.meshgrid(x,y)fig=plt.figure()ax=fig.add_subplot(111,projection='3d')ax.plot_surface(X,Y,Z)# Before fix: offset text gets clipped# After fix: offset text is includedplt.savefig('output.png',bbox_inches='tight')plt.show()

PR checklist

  • "closes[Bug]: axis3d.Axis.get_tightbbox() is not including the offset_text #30744"
  • new and changed code is tested
  • [N/A]Plotting related features are demonstrated in an example (this is a bug fix, not a new feature)
  • [N/A]New Features andAPI Changes are noted with a directive and release note (this is a bug fix that restores expected behavior)
  • [N/A] Documentation complies with general and docstring guidelines (docstring is inherited from parent class)

@FazeelUsmaniFazeelUsmani marked this pull request as draftNovember 18, 2025 14:45
The previous test used an overly strict tolerance (rtol=1e-10) which couldfail due to floating-point precision differences across platforms andbackends. Simplified the test to directly check that the tight bboxcontains the offset text bbox with a reasonable tolerance (1e-6).
The test now only checks that offset_text is included in the tightbbox when it's actually visible and has content. This handles caseswhere different backends or configurations may not show offset text.Added helpful error messages to aid debugging if assertions fail.
@FazeelUsmaniFazeelUsmani marked this pull request as ready for reviewNovember 19, 2025 10:29
@FazeelUsmaniFazeelUsmani marked this pull request as draftNovember 19, 2025 11:45
@FazeelUsmaniFazeelUsmani changed the titleFix axis3d to include offset text in tight bounding box calculation[Wip] Fix axis3d to include offset text in tight bounding box calculationNov 19, 2025
@FazeelUsmaniFazeelUsmaniforce-pushed thefix-axis3d-offset-text-tightbbox branch from27505a1 to1adbbc8CompareNovember 19, 2025 13:49
Match the pattern used for label (line 716) by checking that offsetTexthas actual content before including it in the bounding box calculation.This prevents including empty offset text in the bbox.
@FazeelUsmaniFazeelUsmaniforce-pushed thefix-axis3d-offset-text-tightbbox branch fromd606497 to9335670CompareNovember 19, 2025 13:54
Explicitly close the figure after the test completes to ensureproper cleanup and avoid potential interference with other tests.
Remove explicit plt.close(fig) call as matplotlib's test frameworkhandles cleanup automatically. The explicit close was causing testfailures across multiple platforms.
@FazeelUsmani
Copy link
ContributorAuthor

This PR fixes issue#30744 by includingoffsetText in the 3D axisget_tightbbox() calculation, following the same pattern used in the 2D axis implementation.

Implementation Complete
- Added 2 lines tolib/mpl_toolkits/mplot3d/axis3d.py to include offsetText when visible and non-empty
- Added regression test inlib/mpl_toolkits/mplot3d/tests/test_axes3d.py
- Follows the exact pattern from 2D axis implementation (lib/matplotlib/axis.py:1366-1369)

CI Status:
All relevant tests passing - 30+ successful checks including:
- All Ubuntu x64 tests (Python 3.11, 3.12, 3.13)
- macOS-14 (Python 3.11, 3.12)
- macOS-15 (Python 3.13)
- Windows (Python 3.11, 3.12, 3.13)
- All linting, type checking, and code quality checks
-
Question for Maintainers:
There's one failing check:test_webagg timeout onubuntu-24.04-arm. This test is timing out after 120 seconds in the webagg backend, which is unrelated to the 3D axis changes in this PR.

Is this a known flaky test on ARM runners? Should I:

  1. Proceed as-is since it's unrelated to the changes?
  2. Investigate further?

The core fix is complete and tested. Let me know if any changes are needed!

@FazeelUsmaniFazeelUsmani marked this pull request as ready for reviewNovember 19, 2025 15:55
@FazeelUsmaniFazeelUsmani changed the title[Wip] Fix axis3d to include offset text in tight bounding box calculationFix axis3d to include offset text in tight bounding box calculationNov 19, 2025
@rcomer
Copy link
Member

Hi@FazeelUsmani the webagg test is indeed known to be flaky so you can ignore it.

FazeelUsmani reacted with thumbs up emoji

@FazeelUsmani
Copy link
ContributorAuthor

Thanks@rcomer for confirming! The PR is ready for review.

Copy link
Contributor

@scottshambaughscottshambaugh left a comment

Choose a reason for hiding this comment

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

This looks good for me, thanks for putting it together!

Good candidate for a squash merge

FazeelUsmani reacted with hooray emoji
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@scottshambaughscottshambaughscottshambaugh approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: axis3d.Axis.get_tightbbox() is not including the offset_text

3 participants

@FazeelUsmani@rcomer@scottshambaugh
[8]ページ先頭
©2009-2025 Movatter.jp