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.
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
Add Figure.timestamp to plot the GMT timestamp logo #2208
Add Figure.timestamp to plot the GMT timestamp logo #2208
Changes from 42 commits
c8206ec
2096760
e0bb6bb
4d6d777
fffe3cd
54f4307
fe8eeac
97574ac
1e8de17
79460fb
7031448
7ad112c
b057ad8
f92d272
cecf6de
7dd74d7
06049a6
876d963
752273e
f6795a9
9eba2a9
92c285a
776981b
c771183
386882e
e3580e7
41f5419
4def360
bc64e8c
78c2307
e6248c2
d989b22
960508f
16c0368
98b6762
b8a7fbb
688f6b0
6e168e1
5857866
01a3cbb
4fa2e72
0c07a48
e4bd94c
0079643
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 still unsure what's the best way to document features that require new GMT versions.
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. Is there a similar situation for the Julia wrapper, which can serve as an orientation?
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.
Should a GMTInvalidInput error be raised when users pass
font
as a parameter with GMT 6.3.0? Or is the error clear enough from GMT?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.
The
font
parameter works for any GMT versions.For GMT>6.4.0, it can change both the font ID and font color, but for GMT<=6.4.0, the font color is always black so it can only change the font ID.
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.
Ah ok, maybe reword it into something like this?
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 afraid "font label" is incorrect here. It should be "font ID" or "font 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.
Owh, you mean like font style (e.g. Helvetica)? Feel free to edit the suggested changes.
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.
Added "only the font ID" in 0079643.
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.
The output of the justification image doesn't look correct?
All of the timestamps are on the bottom left (BL). Shouldn't they be at the four different corners?
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.
Unlike other embellishments (e.g., inset, legend or direction rose), the timestamp logo is always located at the lower left corner of the map. I believe this is GMT's design.
See https://docs.generic-mapping-tools.org/dev/gmt.conf.html#term-MAP_LOGO_POS
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.
Interesting design choice 😅 Actually, I just realized I was thinking of
position
rather thanjustification
. Maybe we should document the defaultoffset="-54p/-54p"
under the offset docstring?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.
The default values are clear from the function definition:
Figure.timestamp((text=None, label=None, justification='BL', offset=('-54p', '-54p'), font='Helvetica,black', timefmt='%Y %b %d %H:%M:%S')
I'm hesitate to document the defaults in the docstring.
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.
Ah ok, was thinking of adding
[Default is ('-54p', '-54p')]
, but it's not really needed I guess.