-
Notifications
You must be signed in to change notification settings - Fork 242
Figure.colorbar: Add parameters label/unit/annot/tick/grid and more to set colorbar annotations/labels #4407
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
Merged
+105
−12
Merged
Changes from all commits
Commits
Show all changes
31 commits
Select commit
Hold shift + click to select a range
d8eabe7
Add the Frame/Axis class for setting frame and axes
seisman 87a2407
Merge branch 'main' into class/frame-part1
seisman a184f48
Add to doc index
seisman 3e1ad10
Improve docstrings for 'axes'
seisman 8f3a67c
Fix docstrings
seisman 5fbe28b
Add docstrings for annot/grid/tick
seisman d320711
Add one more test
seisman 0ba431d
Add doctest
seisman b61639a
Add doctests
seisman 3a33364
Fix docstrings
seisman c1dadd6
Fix an edge case
seisman 2dde989
Allow setting annotation angles
seisman e990cbd
Figure.colorbar: Pythonic way to set labels and annotations
seisman 3c75b93
Fix doctest
seisman c946198
Fix typing issue
seisman de43548
Support angle/prefix/unit
seisman 072fde2
Fix a typo
seisman 7aa50bd
Add more doctests
seisman 13f13bf
Fix a typo in doctest
seisman b003726
Fix doctest
seisman a36b687
Merge branch 'main' into class/frame-part1
seisman 03e0ee7
Merge branch 'main' into class/frame-part1
seisman 5832ad9
Merge branch 'class/frame-part1' into colorbar/frame
seisman 68261ed
Fix a typo
seisman c3e7d29
Merge branch 'class/frame-part1' into colorbar/frame
seisman 3f94e41
Fix merge conflicts
seisman 5498316
Allow frame='none' in colorbar
seisman 73bb75f
Merge branch 'main' into colorbar/frame
seisman e30070f
Improve docstrings
seisman e89cc2e
Merge branch 'main' into colorbar/frame
seisman bbbda25
Merge branch 'main' into colorbar/frame
yvonnefroehlich File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is coming from to be consistent with GMT.
I personally find it confusing to call the y-label "unit" in case the frame is related to a colorbar. Thus we have here "annot_unit" for the unit opitionally added after the annotations, but "unit" in case the frame is related to a basemap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also debating which one to choose:
frameparameter, the names should bexlabel/ylabel/annot_prefix/annot_unit/annot_angle(x- and y-label mean labels of long and short dimensions of the colorbar)label/unit/annot_prefix/annot_unit/annot_angle(labelandunitare text strings displayed along the long and short dimensions of a colorbar;unitdoesn't have to be a "unit";unitandannnot_unitmay be more confusing)Or maybe we should call them
xlabel/ylabel/annot_prefix/annot_suffix/annot_angle?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm. Maybe I am already confused by myself 🙃. But do you maybe mean
xlabel/ylabel/prefix/unit/anglehere?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I meant
annot_prefix/annot_unit/annot_angle.While
prefix/unit/anglework well in theAxisclass for theframeparameter, they're too ambiguous for colorbar annotations. In a colorbar context, it's unclear whatprefixmean? I think adding theannot_prefix can make the parameter's purpose clear to understand.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the changes in https://github.com/GenericMappingTools/pygmt-paper-figures/pull/39/changes, only
label/unit/annot/tick/gridare used in the examples, andannot_prefix/annot_unit/annot_angleare not used.So we just need to decide if we want
xlabel/ylableorlabel/unit. We can discuss/implementannot_prefix/annot_unit/annot_anglein a separate PR instead.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I somewhat prefer label/unit because it'll get confusing when you have a vertical colorbar (see elevation example in https://www.pygmt.org/v0.18.0/gallery/embellishments/colorbar.html), in which case xlabel (label) is on the 'y-axis' and ylabel (unit) is on the x-axis. But no strong opinions if we decide to go with xlabel/ylabel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another aspect is, that we have for the
move_textparameter already"label"and"unit"(and"annotations") as arguments. If we use nowxlabelandylabelas parameter names we probably have to adjust the arguments formove_text.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So let's follow the GMT convention and use label/unit. Users need to read the documentation anyway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, with these two arguments, keeping
labelandunitappears better to me.