-
Notifications
You must be signed in to change notification settings - Fork 235
Add contributing guides for wrapping a GMT module #1687
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
Changes from 33 commits
2aee14d
2ace7e9
031460a
293035a
ae143d0
a05a8a7
49538ef
aea5d95
62b8cae
0c170b3
7d26742
2edf308
6dd7d9e
5f3fd30
de77f0c
70fa80c
6063abe
f36dabe
617ec8d
0c32bd2
9fd0cd8
9f38d88
a4f508a
97d6026
ad04b8a
f2a86b8
274a2ca
dc9c0ff
6b467f1
da8a2f6
5a90e11
7361961
a17a641
065fd5e
8bd154b
a2997e0
357fd0d
d753abf
b8e2867
647af3e
5fc2403
eb2c428
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -163,9 +163,11 @@ To increase the chances of getting your pull request accepted quickly, try to: | |
| - Write tests for the code you wrote/modified if needed. | ||
| Please refer to [Testing your code](contributing.md#testing-your-code) or | ||
| [Testing plots](contributing.md#testing-plots). | ||
| - Include an example of new features in the gallery or tutorials. | ||
| Please refer to [Gallery plots](contributing.md#contributing-gallery-plots) | ||
| or [Tutorials](contributing.md#contributing-tutorials). | ||
| - Include an example of new features in the gallery or tutorials. Please refer to | ||
| [Gallery plots](contributing.md#contributing-gallery-plots) or | ||
| [Tutorials](contributing.md#contributing-tutorials). If adding a new | ||
| method/function/class, the gallery example or tutorial should be submitted in a | ||
| separate pull request. | ||
| * Have a good coding style | ||
| - Use readable code, as it is better than clever code (even with comments). | ||
| - Follow the [PEP8](https://pep8.org) style guide for code and the | ||
|
|
@@ -466,14 +468,15 @@ function/class/module/method. | |
|
|
||
| ### PyGMT Code Overview | ||
|
|
||
| The source code for PyGMT is located in the `pygmt/` directory. When contributing | ||
| code, be sure to follow the general guidelines in the | ||
| [pull request workflow](contributing.md#pull-request-workflow) section. | ||
| The source code for PyGMT is located in the `pygmt/` directory. When contributing code, | ||
| please open an issue first to discuss the feature and its implementation and be sure to | ||
| follow the general guidelines in the [pull request workflow](#pull-request-workflow) | ||
| section. | ||
|
|
||
| ### Code Style | ||
|
|
||
| We use the [ruff](https://docs.astral.sh/ruff) tool to format the code, so we | ||
| don't have to think about it. It loosely follow the [PEP8](https://pep8.org) guide | ||
| don't have to think about it. It loosely follows the [PEP8](https://pep8.org) guide | ||
| but with a few differences. Regardless, you won't have to worry about formatting | ||
| the code yourself. Before committing, run it to automatically format your code: | ||
|
|
||
|
|
@@ -511,6 +514,75 @@ contains rules for running the linter checks: | |
| make check # Runs ruff in check mode | ||
| ``` | ||
|
|
||
| ### Wrapping a GMT Module | ||
|
|
||
| Wrapping a GMT module in PyGMT is usually a big task, but it can progress more smoothly | ||
| and efficiently when divided into **small, manageable chunks**. This section gives an | ||
| overview of the main tasks involved. | ||
|
|
||
| 1. Create a feature request ("wrapper request issue") for wrapping a module and discuss | ||
| what users would like to see in the wrapper [optional, usually done by users]. | ||
| 2. Create a tracking issue ("wrapper tracking issue"), using the "Wrapper for a GMT | ||
| module" issue template, to track the progress of wrapping the module. Link it to the | ||
seisman marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| [Project board](https://github.com/orgs/GenericMappingTools/projects/3), and close | ||
| the "wrapper request issue" with a comment such as: | ||
| > Thank you for opening the feature request. The progress of wrapping the module will | ||
| > be tracked in issue #XXX and | ||
| > the [Project board](https://github.com/orgs/GenericMappingTools/projects/3). | ||
|
Comment on lines
528
to
532
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we mention that linking to the project board can only be done by a maintainer? Closing the issue is usually done by the maintainer too.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in 357fd0d |
||
| 3. Open one PR for the initial implementation, focusing on required and essential | ||
| parameters. | ||
| 4. Close the "wrapper tracking issue" once the initial implementation is merged. Leave a | ||
| comment such as: | ||
| > The initial implementation of wrapping the XXX module was completed in PR #XXX. | ||
| > Not all functionalities are implemented yet. Further progress will be tracked in | ||
| > the Project board. | ||
| This is necessary to avoid having hundreds of long-term open issues. | ||
seisman marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| 5. Open one or more PRs to implement the remaining features and missing aliases. | ||
| 6. Open one PR to add a gallery example or a tutorial. | ||
|
|
||
| These PRs can be split among multiple contributors. There is no obligation for a single | ||
| contributor to complete all steps. Please comment on the "wrapper tracking issue" if you | ||
| would like to open a PR for any of these tasks to avoid redundant efforts. | ||
|
|
||
| #### Initial Feature Implementation | ||
|
|
||
| First, comment on the "Wrapper tracking issue" that you will be working on the initial | ||
| implementation. This first pull request should be as minimal as possible - only adding | ||
| the required functionality (i.e., wrapping the required GMT parameters and supporting | ||
| the primary input/output types). | ||
|
|
||
| The following steps are common to all initial implementation pull requests that wrap a | ||
| GMT module: | ||
|
|
||
| * Create a new module `<module-name>.py` in `pygmt/src`. The module docstring should | ||
| include the module name and a short description of the functionality (e.g., | ||
| `grdfill - Fill blank areas from a grid.`). | ||
| * Add a function `<module-name>` to the module. When writing the new function, it is | ||
| generally easiest to reference the source code for other functions that input/output | ||
| similar object types. | ||
| * Write a detailed docstring following the [numpy style guide](https://numpydoc.readthedocs.io/en/latest/format.html). | ||
seisman marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| * Add the function to the import statements in `pygmt/src/__init__.py`. | ||
| * Add the function to the import statements in `pygmt/__init__.py`. | ||
seisman marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| * Add the function to appropriate section of the API documentation in `doc/api/index.rst`. | ||
| * Add a testing module `test_<module-name>.py` in `pygmt/tests`, following | ||
| the guidelines in the [testing your code](#testing-your-code) section. | ||
|
|
||
| #### Add Missing Aliases | ||
|
|
||
| After the initial implementation, missing aliases can be added in separate PRs: | ||
|
|
||
| * Select a suitable alias for each GMT option, following the guidelines in the | ||
| [code style](#code-style) section. Before creating a new alias, check: | ||
|
|
||
| - whether the parameter is listed in the `COMMON_DOCSTRINGS` dictionary in | ||
| `pygmt/helpers/decorators.py` | ||
| - whether other wrapped GMT modules have a similar parameter | ||
| - whether [GMT.jl](https://www.generic-mapping-tools.org/GMT.jl/dev/) has defined an alias | ||
| * Add the alias to the `AliasSystem` class and the function signature. | ||
| * Add the alias and description to the parameters section of the docstring, using the | ||
| `fmt_docstring` decorator to add descriptions for parameters included in the | ||
| `COMMON_DOCSTRINGS` dictionary. | ||
|
|
||
| ### Testing your Code | ||
|
|
||
| Automated testing helps ensure that our code is as free of bugs as it can be. | ||
|
|
@@ -527,7 +599,7 @@ When writing tests, don't test everything that the GMT function already tests, s | |
| the every unique combination arguments. An exception to this would be the most popular | ||
seisman marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
seisman marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| methods, such as <code>pygmt.Figure.plot</code> and <code>pygmt.Figure.basemap</code>. | ||
| The highest priority for tests should be the Python-specific code, such as numpy, | ||
| pandas, and xarray objects and the virtualfile mechanism. | ||
| pandas, and Xarray objects and the virtualfile mechanism. | ||
|
|
||
| If you're **new to testing**, see existing test files for examples of things to do. | ||
| **Don't let the tests keep you from submitting your contribution!** | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.