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.

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

Correct and consistent usage of 'function, 'method', and 'class' #2043

Merged
merged 44 commits into from
Aug 27, 2022

Conversation

yvonnefroehlich
Copy link
Member

@yvonnefroehlich yvonnefroehlich commented Aug 7, 2022

Description of proposed changes

Fixes #2029


Overview of affected files

  • Cross-check in the end regarding completeness

Examples which use 'function' for the methods of the pygmt.Figure class

Examples which use 'method' for the tabular and raster data processing functions

API documentation (list is incomplete and was not continued)

  • figure.py / psconvert
  • table [see below]

I am unsure about the usage of the term 'module'. I remember issue #1828 and PR #1858 which are about whether pygmt.Figure.coast is a 'module' or 'method'.

I am definitely not a Python expert! Probably 'module' is inferred from GMT, but the three listed above do not appear to be 'modules' in the Pythonic-sense from xy_module import xy_function (see also second paragraph of issue #1828 (comment)). Maybe it was overlook to also change the API doc table in PR #1858. But why they are 'methods', as changed in PR #1858, despite they do not belong to a class or called on an object? I would be very happy about an explanation for understanding (: In case they are neither 'modules' nor 'methods', it appears that they are 'functions'. Ping @seisman, @maxrjones, @michaelgrund, @willschlitzer (, and @weiji14) for thoughts and help on this, please.


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 wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs.
  • 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

@yvonnefroehlich yvonnefroehlich changed the title Correct and consistent usage of 'function' and 'method' WIP: Correct and consistent usage of 'function' and 'method' Aug 7, 2022
@maxrjones
Copy link
Member

Thanks for working on this!

While there could be exceptions, I think the use of module anywhere in PyGMT's docs likely comes from GMT-centric language and not from any Python-related definition. So unless specifically referring to GMT, these could be updated to "function" or "method".

You are correct that pygmt.set_display, pygmt.grd2cpt, and pygmt.makecpt are functions.

@seisman seisman added the documentation Improvements or additions to documentation label Aug 8, 2022
@seisman seisman added this to the 0.8.0 milestone Aug 8, 2022
@yvonnefroehlich
Copy link
Member Author

Thanks for working on this!

I am happy to contribute, and at least partly I feel I learn more than I bring to the project 😉

While there could be exceptions, I think the use of module anywhere in PyGMT's docs likely comes from GMT-centric language and not from any Python-related definition. So unless specifically referring to GMT, these could be updated to "function" or "method".

Thanks @maxrjones for your explanation! 🙂

You are correct that pygmt.set_display, pygmt.grd2cpt, and pygmt.makecpt are functions.

I have made the changes in b313a67 as well as 0b13099 and 80339eb. Try to continue with the remaining examples in the next days.

@yvonnefroehlich yvonnefroehlich changed the title WIP: Correct and consistent usage of 'function' and 'method' Correct and consistent usage of 'function' and 'method' Aug 20, 2022
@yvonnefroehlich
Copy link
Member Author

I have now updated all examples listed above.

@seisman
Copy link
Member

seisman commented Aug 20, 2022

@yvonnefroehlich Please resolve the conflicts first.

Copy link
Member

@seisman seisman left a comment

Choose a reason for hiding this comment

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

Looks great!

@seisman seisman added the final review call This PR requires final review and approval from a second reviewer label Aug 20, 2022
@yvonnefroehlich
Copy link
Member Author

In first_figure.py:

To add to a plot object (fig in this example), the PyGMT module is used as a method on the class. This example will use the pygmt.Figure.coast method, which can be used to create a map without any other methods, modules or external data. The pygmt.Figure.coast method plots the coastlines, borders, and bodies of water using a database that is included in GMT.

I am unsure about the formulations in this part. Should we keep modules in ... other methods, modules or external data ... or change it to functions? PyGMT itself does not have modules in a Pythonic sense. But maybe "modules" is referring to numpy, pandas, etc.?

@seisman
Copy link
Member

seisman commented Aug 23, 2022

In first_figure.py:

To add to a plot object (fig in this example), the PyGMT module is used as a method on the class. This example will use the pygmt.Figure.coast method, which can be used to create a map without any other methods, modules or external data. The pygmt.Figure.coast method plots the coastlines, borders, and bodies of water using a database that is included in GMT.

I am unsure about the formulations in this part. Should we keep modules in ... other methods, modules or external data ... or change it to functions? PyGMT itself does not have modules in a Pythonic sense. But maybe "modules" is referring to numpy, pandas, etc.?

I think it originally means "GMT's modules".

Because this is the first PyGMT tutorial and the potential PyGMT users may have never used GMT before, I think it makes very little sense to mention any GMT concepts (e.g., modules) here. So, it's OK to say "without any other methods or external data."

@seisman
Copy link
Member

seisman commented Aug 23, 2022

To add to a plot object (fig in this example), the PyGMT module is used as a method on the class.

I also feel that this sentence is a little confusing, because "the PyGMT module" actually means "GMT modules".

@yvonnefroehlich
Copy link
Member Author

In first_figure.py:

To add to a plot object (fig in this example), the PyGMT module is used as a method on the class. This example will use the pygmt.Figure.coast method, which can be used to create a map without any other methods, modules or external data. The pygmt.Figure.coast method plots the coastlines, borders, and bodies of water using a database that is included in GMT.

I am unsure about the formulations in this part. Should we keep modules in ... other methods, modules or external data ... or change it to functions? PyGMT itself does not have modules in a Pythonic sense. But maybe "modules" is referring to numpy, pandas, etc.?

I think it originally means "GMT's modules".

Because this is the first PyGMT tutorial and the potential PyGMT users may have never used GMT before, I think it makes very little sense to mention any GMT concepts (e.g., modules) here. So, it's OK to say "without any other methods or external data."

Done in 3e91e11.

To add to a plot object (fig in this example), the PyGMT module is used as a method on the class.

I also feel that this sentence is a little confusing, because "the PyGMT module" actually means "GMT modules".

I agree. I already have tried to reformulate this sentence, but I was unsure. Thanks @seisman for this explanation! I have made a suggestion in 3db83c8. Of course, I am totally open for changes and improvements on this.

@seisman
Copy link
Member

seisman commented Aug 27, 2022

@yvonnefroehlich Reviewing big PRs is more challenging and takes more time than small PRs. I think this PR already has a lot of good changes and should be merged into the main branch after reviewing.

If you think there are still many files that need to be updated, you can open separate PRs later, like what we did in PRs #1857, #1862, #1872.

@yvonnefroehlich
Copy link
Member Author

yvonnefroehlich commented Aug 27, 2022

No worries. We'll give it another round of review when you finish it/

Thanks for your understanding @seisman!

@yvonnefroehlich Reviewing big PRs is more challenging and takes more time than small PRs. I think this PR already has a lot of good changes and should be merged into the main branch after reviewing.

If you think there are still many files that need to be updated, you can open separate PRs later, like what we did in PRs #1857, #1862, #1872.

That's fine for me! 😉 I added the last two changes I had on my list for now.

@yvonnefroehlich yvonnefroehlich changed the title WIP: Correct and consistent usage of 'function, 'method', and 'class' Correct and consistent usage of 'function, 'method', and 'class' Aug 27, 2022
@seisman seisman added the final review call This PR requires final review and approval from a second reviewer label Aug 27, 2022
@seisman seisman merged commit b70080a into main Aug 27, 2022
@seisman seisman deleted the correct-usage-function-method branch August 27, 2022 17:06
@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Nov 23, 2022
@weiji14 weiji14 added the skip-changelog Skip adding Pull Request to changelog label Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation skip-changelog Skip adding Pull Request to changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent usage of 'function' and 'method'
5 participants