Skip to content

Conversation

@cvanelteren
Copy link
Collaborator

@cvanelteren cvanelteren commented Jul 3, 2025

minor adjustment of #301

This PR is now a refactor of how the label positioning is handled. It moves the ugly block to a much easier to read helper function with some internal checking.

Important changes:

  • label can now be placed independent of the orientation
  • the label will now be rotated -90 degree for vertical colorbars by default to ensure visual fidelity across the kinds of plots (inset, figure, or axis).

I will need to add some tests to test this properly and still adjust the bbox handling with this new approach.

@cvanelteren cvanelteren enabled auto-merge (squash) July 3, 2025 16:07
Copy link
Collaborator

@beckermr beckermr left a comment

Choose a reason for hiding this comment

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

Do we want to add a test for this by adding an image to our image comparison tests?

@cvanelteren cvanelteren disabled auto-merge July 3, 2025 16:09
@cvanelteren
Copy link
Collaborator Author

Yes. Hold on. It's a bit tricky as the vert bars suffer from looking bad if you open the figure in gtk but look good on save

@cvanelteren
Copy link
Collaborator Author

Was drafting a release and noticed this

@codecov
Copy link

codecov bot commented Jul 3, 2025

Codecov Report

❌ Patch coverage is 86.53846% with 21 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
ultraplot/axes/base.py 84.25% 10 Missing and 10 partials ⚠️
ultraplot/tests/test_colorbar.py 96.55% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@cvanelteren cvanelteren marked this pull request as draft July 3, 2025 16:26
@cvanelteren cvanelteren changed the title adjust value slighlty Refactor colorbar loc handling Jul 4, 2025
@cvanelteren
Copy link
Collaborator Author

Need to fix the framing for the inset

@cvanelteren
Copy link
Collaborator Author

Ok I sat down and went through all the edge cases but I think I got it looking relatively nice now. I am afraid, however, that the tests will indicate a visual difference since the logic is now concerning all the locations for insets.

@cvanelteren cvanelteren requested a review from beckermr July 6, 2025 15:11
@cvanelteren
Copy link
Collaborator Author

The resulting tests look good but differ in shape. Not sure why but likely related to the newly added refactors. Otherwise visually looks good.

@cvanelteren
Copy link
Collaborator Author

See above for changes @beckermr

@cvanelteren cvanelteren marked this pull request as ready for review July 6, 2025 18:09
@cvanelteren
Copy link
Collaborator Author

Can we go ahead an merge this @beckermr? I manually checked the errors and there is an image dimension error but visually they look similar.

@beckermr
Copy link
Collaborator

yes merge away!

@cvanelteren cvanelteren enabled auto-merge (squash) July 30, 2025 17:01
@cvanelteren cvanelteren disabled auto-merge July 30, 2025 17:01
@cvanelteren cvanelteren merged commit 5f11343 into Ultraplot:main Jul 30, 2025
15 of 25 checks passed
@cvanelteren cvanelteren deleted the adjust-vert-cbar-inset-space branch July 30, 2025 17:01
cvanelteren added a commit that referenced this pull request Aug 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants