Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Figure.plot: Deprecate parameter "sizes" to "size" (remove in v0.6.0) #1254

Merged
merged 8 commits into from
May 6, 2021
Merged

Figure.plot: Deprecate parameter "sizes" to "size" (remove in v0.6.0) #1254

merged 8 commits into from
May 6, 2021

Conversation

seisman
Copy link
Member

@seisman seisman commented May 3, 2021 •
edited
Loading

Description of proposed changes

  • Rename the parameter sizes to size in Figure.plot()
  • Updates tests and examples to use the new parameter name
  • Use the deprecate_parameter decorator for backward-compatibility
  • Add a test for checking 'sizes' backward compatibility

Address #1118.

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 added the deprecation Deprecating a feature label May 3, 2021
@seisman seisman added this to the 0.4.0 milestone May 3, 2021
@seisman seisman requested a review from core-man May 3, 2021 03:58
core-man
core-man approved these changes May 3, 2021
View reviewed changes
Copy link
Member

@core-man core-man left a comment

Choose a reason for hiding this comment

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

All are good except one minor comment.

examples/tutorials/plot.py Show resolved Hide resolved
michaelgrund
michaelgrund approved these changes May 3, 2021
View reviewed changes
Copy link
Member

@michaelgrund michaelgrund left a comment

Choose a reason for hiding this comment

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

Except the comment provided by @core-man, everything looks fine to me!

weiji14
weiji14 reviewed May 3, 2021
View reviewed changes
pygmt/src/plot.py Outdated Show resolved Hide resolved
pygmt/src/plot.py Outdated Show resolved Hide resolved
pygmt/tests/test_plot.py Show resolved Hide resolved
examples/tutorials/plot.py Show resolved Hide resolved
@seisman seisman added the final review call This PR requires final review and approval from a second reviewer label May 4, 2021
@seisman seisman changed the title Figure.plot: Deprecate the parameter "sizes" to "size" Figure.plot: Deprecate the parameter "sizes" to "size" (will be removed in v0.6.0) May 4, 2021
@seisman seisman changed the title Figure.plot: Deprecate the parameter "sizes" to "size" (will be removed in v0.6.0) Figure.plot: Deprecate parameter "sizes" to "size" (will be removed in v0.6.0) May 4, 2021
@seisman
Copy link
Member Author

seisman commented May 4, 2021

Since the PR title will be used in the release notes, I just changed the PR title to

Figure.plot: Deprecate parameter "sizes" to "size" (will be removed in v0.6.0)

to make it more readable.

I expect to have a "consistent" PR title for parameter deprecations like this, so suggestions on the PR title format are also welcomed.

@seisman seisman changed the title Figure.plot: Deprecate parameter "sizes" to "size" (will be removed in v0.6.0) Figure.plot: Deprecate parameter "sizes" to "size" (fully removed in v0.6.0) May 6, 2021
@seisman seisman changed the title Figure.plot: Deprecate parameter "sizes" to "size" (fully removed in v0.6.0) Figure.plot: Deprecate parameter "sizes" to "size" (removed in v0.6.0) May 6, 2021
@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label May 6, 2021
weiji14
weiji14 approved these changes May 6, 2021
View reviewed changes
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.

Since the PR title will be used in the release notes, I just changed the PR title to

Figure.plot: Deprecate parameter "sizes" to "size" (will be removed in v0.6.0)

to make it more readable.

I expect to have a "consistent" PR title for parameter deprecations like this, so suggestions on the PR title format are also welcomed.

Looks good. I would just change the 'removed' to 'remove' in the title, i.e. 'Figure.plot: Deprecate parameter "sizes" to "size" (remove in v0.6.0)'

weiji14
weiji14 reviewed May 6, 2021
View reviewed changes
pygmt/tests/test_plot.py Outdated Show resolved Hide resolved
@seisman seisman changed the title Figure.plot: Deprecate parameter "sizes" to "size" (removed in v0.6.0) Figure.plot: Deprecate parameter "sizes" to "size" (remove in v0.6.0) May 6, 2021
seisman
seisman commented May 6, 2021
View reviewed changes
pygmt/tests/test_plot.py Outdated Show resolved Hide resolved
@seisman @weiji14
Apply suggestions from code review
64c7631 
Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com>
@seisman seisman merged commit b51e3c2 into master May 6, 2021
@seisman seisman deleted the deprecate/plot-sizes branch May 6, 2021 22:26
@weiji14 weiji14 mentioned this pull request Mar 13, 2022
Merged
6 tasks
weiji14 added a commit that referenced this pull request Mar 13, 2022
@weiji14
Figure.plot/plot3d: Remove parameter "sizes", use "size" instead (#1809)
9b1d516 
Remove the parameter "sizes" in favour of "size" from `Figure.plot`
and `Figure.plot3d`. Deprecation warning was added in v0.4.0
(xref #1254, #1258), and is to be fully removed in v0.6.0.
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
@seisman @weiji14
Figure.plot: Deprecate parameter "sizes" to "size" (remove in v0.6.0) (…
4ad95e4 
…GenericMappingTools#1254)

* Rename sizes to size in Figure.plot()
* Updates tests and examples to use the new parameter name
* Use the deprecate_parameter decorator for backward-compatibility
* Add a test for checking 'sizes' backward compatibility

Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com>
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
@weiji14
Figure.plot/plot3d: Remove parameter "sizes", use "size" instead (Gen…
b0a7682 
…ericMappingTools#1809)

Remove the parameter "sizes" in favour of "size" from `Figure.plot`
and `Figure.plot3d`. Deprecation warning was added in v0.4.0
(xref GenericMappingTools#1254, GenericMappingTools#1258), and is to be fully removed in v0.6.0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weiji14 weiji14 weiji14 approved these changes

@core-man core-man core-man approved these changes

@michaelgrund michaelgrund michaelgrund approved these changes

Assignees
No one assigned
Labels
deprecation Deprecating a feature
Projects
None yet
Milestone
0.4.0
Development

Successfully merging this pull request may close these issues.
4 participants
@seisman @michaelgrund @weiji14 @core-man