-
Notifications
You must be signed in to change notification settings - Fork 224
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
Wrap text #321
Wrap text #321
Conversation
Initial commit for wrapping the text/pstext function raised at GenericMappingTools#122, to be implemented under base_plotting.py alongside the other mapping related stuff. Original GMT `text` documentation can be found at https://gmt.soest.hawaii.edu/doc/latest/text.html. Storing sample test cases at test_text.py. Current implementation takes in either a text filename or x,y,text triples as input.
Extra angle, font, justify arguments to fine-tune how the text looks. These arguments all fall under the '-F' parameter, and are set by appending '+a', '+f' or '+j' respectively. Also reduce number of local variables to avoid "R0914: Too many local variables" lint error.
Checks that pygmt.text can justify the text when setting justify=True (using a boolean instead of string ""), reading the option from a column inside a textfile. Example is adapted from the "All great-circle paths lead to Rome" gallery example at https://gmt.soest.hawaii.edu/doc/latest/gallery/ex23.html.
@weiji14 this looks good! I would leave multiple text files for a later PR if needed. I've been thinking if it even makes sense to be allowing filename inputs to all these methods in the first place (but that is for another discussion). |
if justify is not None and isinstance(justify, str): | ||
kwargs["F"] += f"+j{justify}" | ||
|
||
with GMTTempFile(suffix=".txt") as tmpfile: |
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.
We should look into passing text though virtual files, actually. I was avoiding this a while ago because the API had changed but that has been standardized now in 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.
Sorry, I don't quite get your intention here? Do you mean passing text
(as in the command) through virtual files or 'text' (as in plain old text)? Either way I'm confused...
This shouldn't be too hard now that #322 is merged in. Should be a matter of setting it in the function decorator (i.e.
Yes we should (allow filename inputs), for at least 2 reasons. 1) |
Using the sequence_space processor in #325 to allow for passing in a list of textfiles to be plotted! Problem with that `text` would consider a numpy matrix as a list to be parsed, so the code was updated to fix that. Unit tests and docstrings updated. Strangely enough, the 'projection'/'-J' argument doesn't seem to be required anymore? Also regenerated baseline plots to use their rightful name...
@weiji14 what do you think about adding this to the 0.1.0 milestone? It seems pretty integral. |
Sure, why don't you review it and see if there's any issues 😄 I'll update the branch now. |
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.
Overall looks great, just the inconsistency with fig.plot()
that we might want to address!
Cool, thanks for reviewing @liamtoney! |
Adding extra functionality to text, including aliases that were missing in initial implementation at #321, in support of the text tutorial at #480. Allow passing str type into angle argument of text. Aliased pen(W), clearance(C), fill(G), and offset(D). Enable text placement using position (`-F+c`) argument instead of x/y pairs. Reorder docstring to say 'angle, font, justify' instead of 'font, angle, justify'. Added test_text_position to plot at all 9 possible positions. Removed check for missing file in textfiles, and checked that plotting text from an external file (@Table_5_11.txt) works. Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
Description of proposed changes
Wrapping the
text
mapping function! The way I've implemented is that a user can either add a single line of text easily like so:or multiple lines like so:
And of course, advanced users can still pass in
textfiles
instead of using Python/NumPy objects.TODO:
Fixes #122, #319
Reminders
make format
andmake check
to make sure the code follows the style guide.doc/api/index.rst
.