Skip to content

Refactor colorbar loc handling #304

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

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

cvanelteren
Copy link
Contributor

@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
Contributor 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
Contributor Author

Was drafting a release and noticed this

Copy link

codecov bot commented Jul 3, 2025

Codecov Report

Attention: 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
Contributor Author

Need to fix the framing for the inset

@cvanelteren
Copy link
Contributor 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
Contributor 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
Contributor Author

See above for changes @beckermr

@cvanelteren cvanelteren marked this pull request as ready for review July 6, 2025 18:09
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