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 zlabel on 3D axes being cut off#28576

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
juanis2112 wants to merge2 commits intomatplotlib:main
base:main
Choose a base branch
Loading
fromjuanis2112:fix-zlabel

Conversation

juanis2112
Copy link
Contributor

PR summary

Close issue#28117

This PR tries to imitated the behaviour for adding labels with 2D plots in 3D plots. I also updated the png for the corresponding test.

This is a screenshot of the fixed issue:

Screenshot 2024-07-14 at 2 29 46 PM

PR checklist

@QuLogicQuLogic added the mentored: sprintIssues intended and suitable for sprints labelJul 14, 2024
@QuLogic
Copy link
Member

From a subjective view, the test image looks worse. However, I think objectively, it is more correct in that it is doing tight layout, and the z label really is far away.

@scottshambaugh is there some way we can get the labels to not be so far away from the Axes once it is closer to head on?

@juanis2112
Copy link
ContributorAuthor

@QuLogic does this need a rebase for the circleci tests to pass or is it something else?

@tacaswell
Copy link
Member

Yes, needs a rebase.

@oscargus
Copy link
Member

Can you please add a test that generates the image that you show above?

@scottshambaugh
Copy link
Contributor

Right now the label offsets are hardcoded without an rcparam. The labels are drawn here:

def_draw_labels(self,renderer,edgep1,edgep2,labeldeltas,centers,dx,dy):
label=self._axinfo["label"]
# Draw labels
lxyz=0.5* (edgep1+edgep2)
lxyz=_move_from_center(lxyz,centers,labeldeltas,self._axmask())
tlx,tly,tlz=proj3d.proj_transform(*lxyz,self.axes.M)
self.label.set_position((tlx,tly))
ifself.get_rotate_label(self.label.get_text()):
angle=art3d._norm_text_angle(np.rad2deg(np.arctan2(dy,dx)))
self.label.set_rotation(angle)
self.label.set_va(label['va'])
self.label.set_ha(label['ha'])
self.label.set_rotation_mode(label['rotation_mode'])
self.label.draw(renderer)

Withlabeldeltas being calculated here:

# Calculate offset distances
# A rough estimate; points are ambiguous since 3D plots rotate
reltoinches=self.figure.dpi_scale_trans.inverted()
ax_inches=reltoinches.transform(self.axes.bbox.size)
ax_points_estimate=sum(72.*ax_inches)
deltas_per_point=48/ax_points_estimate
default_offset=21.
labeldeltas= (self.labelpad+default_offset)*deltas_per_point*deltas

In generalaxis3d is not in amazing shape and needs a good bit of attention to clean up and make customizable & robust. But in the meantime for the fix in this PR, I'm reticent to accept a change that messes with tight layout subplots so much.

@scottshambaugh
Copy link
Contributor

Opened an issue for controlling the label positions#28606, but I think it's a bit separate from the issue at discussion here.

@juanis2112
Copy link
ContributorAuthor

@QuLogic I don't think the test failing is related to my changes. I re based and force pushed. Let me know if there is anything else to do for this PR :)

@scottshambaugh
Copy link
Contributor

scottshambaugh commentedAug 9, 2024
edited
Loading

I personally don't think this change should be made until there is a way to avoid the super-compressed subplots as seen in the test image, and I don't think that controlling the axis label position like my comment above is a robust way to do so. Not sure what that solution looks like, but would want to see that update made in order for this fix to go in.

oscargus reacted with thumbs up emoji

Copy link
Contributor

Choose a reason for hiding this comment

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

Request that a fix for this image change be made prior to merging in the original fix.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@scottshambaughscottshambaughscottshambaugh requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
mentored: sprintIssues intended and suitable for sprintstopic: mplot3d
Projects
Status: Needs review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

5 participants
@juanis2112@QuLogic@tacaswell@oscargus@scottshambaugh

[8]ページ先頭

©2009-2025 Movatter.jp