NotificationsYou must be signed in to change notification settings
Fork8.1k
Star22.1k

ENH: add long_axis property to colorbar#27167

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

Jump to bottom

Merged

jklymak merged 4 commits intomatplotlib:mainfromjklymak:enh-expose-long-axis

Oct 28, 2024

Merged

ENH: add long_axis property to colorbar#27167

jklymak merged 4 commits intomatplotlib:mainfromjklymak:enh-expose-long-axis

Oct 28, 2024

Conversation

Copy link

Member

jklymak commentedOct 22, 2023•
edited
Loading

PR summary

As discussed in#26896 exposing a long_axis property for colorbar.

PR checklist

"closes #0000" is in the body of the PR description tolink the related issue
new and changed code istested
Plotting related features are demonstrated in anexample
New Features andAPI Changes are noted with adirective and release note
Documentation complies withgeneral anddocstring guidelines

jklymak added the topic: color/colorbar label

Oct 22, 2023

jklymak requested a review fromanntzer

October 22, 2023 23:39

anntzer reviewed

Oct 23, 2023

View reviewed changes

lib/matplotlib/colorbar.py Outdated


		@property
		deflong_axis(self):
		"""Axis that has decorations (ticks etc) on it."""

Copy link

Contributor

anntzerOct 23, 2023

Choose a reason for hiding this comment

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

comma before etc.

anntzer approved these changes

Oct 23, 2023

View reviewed changes

Copy link

Contributor

anntzer left a comment

Choose a reason for hiding this comment

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

Looks fine to me; we could consider replacing all internal uses of _long_axis() by the public long_axis but there's no urgency to do that.

Copy link

MemberAuthor

jklymak commentedOct 24, 2023

I think I replaced all the _long_axis calls.

Copy link

Member

timhoffm commentedOct 24, 2023

Can we please still have a short discussion on naming? See#26896 (comment)

story645 reviewed

Oct 24, 2023

View reviewed changes

Copy link

Member

story645 left a comment•
edited
Loading

Choose a reason for hiding this comment

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

Blocking to prevent accidental merge until this is discussed per@timhoffm's comments

I don't love value or color cause those are the fill values of the colorbar (texture could be added instead of color) and not exactly the axis of the bar frame.

story645 previously requested changes

Oct 24, 2023

View reviewed changes

Copy link

Member

story645 left a comment

Choose a reason for hiding this comment

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

Forgot to actually select request changes 🤦‍♀️

Copy link

Member

timhoffm commentedSep 20, 2024

Let's move this forward. I still favordata_axis because it captures the semantic information aspect rather than just geometry. I'd be happy if you would see a value in switching to that.

I just started to likedata_axis. Whiledata is a very generic term and should usually be avoided, maybe we actually want that generic aspect here. We don't need to focus on the exact type of data (one can think label, color, numeric value as one likes).data generically captures thesemantic information aspect, contrasting from the othernon-data axis.

But if not, let's not hold this up longer.