Skip to content

Conversation

seisman
Copy link
Member

@seisman seisman commented Jun 2, 2021

Description of proposed changes

Address the comment in #1203 (comment).

When long-form parameters are available, we should recommend users not to use
the short-form parameters.

For example, running the following example:

import pygmt

fig = pygmt.Figure()
fig.basemap(region=[0, 10, 0, 10], B=True, J="X10c")
fig.savefig("map.pdf")

will raise warnings like:

test.py:4: FutureWarning: Short-form parameter (J) is not recommended. Use long-form parameter 'projection' instead.
  fig.basemap(region=[0, 10, 0, 10], B=True, J="X10c")
test.py:4: FutureWarning: Short-form parameter (B) is not recommended. Use long-form parameter 'frame' instead.
  fig.basemap(region=[0, 10, 0, 10], B=True, J="X10c")

Reminders

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst.
  • Write detailed docstrings for all functions/methods.
  • If adding new functionality, add an example to docstrings or tutorials.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash commands are:

  • /format: automatically format and lint the code
  • /test-gmt-dev: run full tests on the latest GMT development version

@seisman seisman force-pushed the warn-short-form-parameters branch from 557f16c to 2938c2a Compare June 2, 2021 21:10
@weiji14 weiji14 changed the title Raise an warning for the use of short-form parameters when long-forms are available Raise a warning for the use of short-form parameters when long-forms are available Jun 2, 2021
@seisman seisman added this to the 0.4.0 milestone Jun 3, 2021
@seisman seisman added the deprecation Deprecating a feature label Jun 3, 2021
@seisman seisman marked this pull request as ready for review June 3, 2021 16:50
@seisman seisman requested a review from a team June 6, 2021 21:42
@maxrjones maxrjones mentioned this pull request Jun 15, 2021
27 tasks
Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok to approve. Technically we should have written a test for this, but there are actually some tests still using short parameter names (as mentioned in #1316 (comment)) so I think it's alright to just merge it in as is.

@weiji14 weiji14 added the final review call This PR requires final review and approval from a second reviewer label Jun 16, 2021
@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Jun 17, 2021
@seisman seisman merged commit 3be69e4 into master Jun 17, 2021
@seisman seisman deleted the warn-short-form-parameters branch June 17, 2021 02:43
weiji14 added a commit to weiji14/deepicedrain that referenced this pull request Jun 29, 2021
Silences the SyntaxWarnings raised in PyGMT v0.4.0 due to GenericMappingTools/pygmt#1316.
@weiji14 weiji14 mentioned this pull request Aug 30, 2021
5 tasks
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation Deprecating a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants