-
Notifications
You must be signed in to change notification settings - Fork 214
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow passing arguments containing spaces into pygmt functions #1487
Merged
Merged
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
b5ad405
Replace spaces in arguments with octal code 040
weiji14 14648c2
Remove workarounds for spaces in fig.subplot's autolabel and title args
weiji14 924f439
Remove workaround for spaces in fig.text's -F argument
weiji14 707745f
Remove double quotes around legend label test examples
weiji14 03e4308
Edit test_rose_no_sectors to remove single quotes from title
weiji14 095449a
Remove workaround for spaces in fig.psconvert prefix
weiji14 d81b80b
Format using blackdoc and fix flake8 error
weiji14 dd849cb
Merge branch 'main' into arg_with_space
weiji14 c29e632
Ensure spaces in pygmt.config arguments can work
weiji14 83c8c3b
Manually handle prefix -F in psconvert
weiji14 2ba8420
Merge branch 'main' into arg_with_space
weiji14 b58af8b
Merge branch 'main' into arg_with_space
weiji14 3ec7727
Handle PROJ4 strings with spaces
weiji14 f19c41b
Use Modifier Letter Colon instead of regular colon to fix WIndows tests
weiji14 7a518a1
Merge branch 'main' into arg_with_space
weiji14 9774d5e
Try using underscore instead of Modifier Letter Colon
weiji14 ff40d27
Refactor the whole function using a single loop
weiji14 36fdec5
Merge branch 'main' into arg_with_space
weiji14 6c399ba
Merge branch 'main' into arg_with_space
weiji14 4770396
Merge branch 'main' into arg_with_space
weiji14 801ba01
Raise GMTInvalidInput if no prefix argument is passed to psconvert
weiji14 ecb580e
Merge branch 'main' into arg_with_space
weiji14 a526d45
Fix rst syntax in pygmt/helpers/utils.py
weiji14 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
Manually handle prefix -F in psconvert
So that fig.savefig won't insert `\040` characters when saving filenames with spaces. Resolves problem mentioned in https://github.com/GenericMappingTools/pygmt/pull/1487/files#r703116544
- Loading branch information
commit 83c8c3b0e8c699cd52e7d539c49dabbbf7bf384c
There are no files selected for viewing
This file contains 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
This file contains 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
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.
Should there be a check before this that
prefix
is provided so that the user doesn't get a KeyError here? Or you could raise GMTInvalidInput in an except statement.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.
Good point. Looking at the docs at https://docs.generic-mapping-tools.org/6.3/psconvert.html, it reads like
prefix
/-F
is optional, but when I tried runningpsconvert
without the-F
option, it throws an error:psconvert [ERROR]: Modern GMT mode requires the -F option
. See https://github.com/GenericMappingTools/gmt/blob/adb244afa51ca7246cc051080c9d47193087d6c2/src/psconvert.c#L1041-L1042So since PyGMT is modern mode only, it should be fine to keep this line intact.
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 docs say "If no input files are given, will convert the current active figure (see pygmt.figure). In this case, an output name must be given using parameter prefix." How does one provide other input files than the current figure using pygmt.Figure.psconvert?
If we chose not to support operations analogous to
psconvert -Tg test.ps
(which uses the same prefix as the original file name), I think we should makeprefix
required in the function signature or raise an invalid input exception to avoid the traceback below. I don't think it's obvious to that the error is caused by not usingprefix
.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 can't, because it has not been implemented yet. The current
fig.psconvert
is a method of thepygmt.Figure()
class and is tied to thefig
, and can only 'psconvert' the active figure.Ok, I've added a try-except block in 801ba01 to raise a GMTInvalidInput if
prefix
/-F
is not given.