-
Notifications
You must be signed in to change notification settings - Fork 235
Add Figure.scalebar to plot a scale bar on maps #4015
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
Conversation
7a276b0 to
ec9b800
Compare
8acb461 to
14283bd
Compare
2a0ea6b to
17d8427
Compare
Summary of changed imagesThis is an auto-generated report of images that have changed on the DVC remote
Image diff(s)DetailsAdded images
Modified images
Report last updated at commit cc40c8f |
| length: float | str | None = None, | ||
| scale_position: float | Sequence[float] | bool = False, | ||
| label: str | bool = False, | ||
| label_alignment: Literal["left", "right", "top", "bottom"] | None = None, |
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.
GMT and GMT.jl use align, currently PyGMT uses label_alignment, but maybe label_position?
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 feel label_position is more consisten, as we have nan_position for Figure.colorbar. And it is a bit shorter.
0fdd8a3 to
c86e8d5
Compare
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.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
pygmt/src/scalebar.py
Outdated
| length: float | str, | ||
| height: float | str | None = None, | ||
| position: Position | Sequence[float | str] | AnchorCode | None = None, | ||
| scale_at: float | Sequence[float] | bool = False, |
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. I do not really like this name.
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.
GMT uses loc, GMT.jl uses scale_at_lat.
An alternative name may be scale_loc?
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.
Not so easy finding a good name here. I was thinking about something in the direction of
reference_coords: a bit longreference_lat,scale_lat: partly also the longitude is given
Maybe location is enough, as "scale" is already contained in the method name "scalebar"?
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 feel users may be confused by position and location
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, this is something I was also a bit unsure about. Maybe there are ideas or preferences by @weiji14 and @michaelgrund?
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 scale_loc is the best name we have so far:
- not too long
- location can be all, latitude, longitude, middle of the map, reference point
- "loc" as a shortcut for "location" should be OK
- the leading "scale" should reduce a potential confusion with
position
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.
OK. Done in 0561ba4.
Co-authored-by: Yvonne Fröhlich <94163266+yvonnefroehlich@users.noreply.github.com>
Co-authored-by: Yvonne Fröhlich <94163266+yvonnefroehlich@users.noreply.github.com>
Co-authored-by: Yvonne Fröhlich <94163266+yvonnefroehlich@users.noreply.github.com>
Co-authored-by: Yvonne Fröhlich <94163266+yvonnefroehlich@users.noreply.github.com>
|
I'll merge this PR in 24 hours if no further comments. |
|
The gallery example (https://www.pygmt.org/dev/gallery/embellishments/scalebar.html) is updated in a separate PR, like for |
Yes, too keep this PR small |
| length: float | str | None = None, | ||
| scale_position: float | Sequence[float] | bool = False, | ||
| label: str | bool = False, | ||
| label_alignment: Literal["left", "right", "top", "bottom"] | None = None, |
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 feel label_position is more consisten, as we have nan_position for Figure.colorbar. And it is a bit shorter.
yvonnefroehlich
left a comment
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.
Made two more comments.
Besides this, this PR looks good to me.
Co-authored-by: Yvonne Fröhlich <94163266+yvonnefroehlich@users.noreply.github.com>
|
|
I'm merging this PR. We can still change the parameter names before releasing v0.19.0. |
OK, consistency with |
@yvonnefroehlich Please let me know if you're interested in updating the gallery example https://www.pygmt.org/dev/gallery/embellishments/scalebar.html. |


This PR implements the
Figure.scalebarmethod, which wraps the-Loption inbasemap/coast. Related to #4268.The full syntax of -L is:
Here is a comparison of names used in PyGMT, GMT, and GMT.jl:
Preview: https://pygmt-dev--4015.org.readthedocs.build/en/4015/api/generated/pygmt.Figure.scalebar.html#pygmt.Figure.scalebar
Related to #4268
TODO in separate PRs
scalebarmethodmap_scaleinFigure.coast/Figure.basemapFigure.basemap/Figure.coast: Recommend to use the Figure.scalebar method to add a scalebar #4337