-
Notifications
You must be signed in to change notification settings - Fork 229
Description
Description of the desired feature
Wrapping a new GMT module in PyGMT is usually a big task (as we've learned the hard way from #804), so it's better on both the contributor and reviewer(s) if small, manageable chunks are done (as per https://github.com/GenericMappingTools/pygmt/blob/v0.5.0/doc/contributing.md#general-guidelines-for-making-a-pull-request-pr).
This step-by-step process seems to have worked well for @willschlitzer and others in the past few months, and for the sake of new contributors, we should probably 1) document the step-by-step policy and 2) track the status of each GMT module being wrapped.
Documenting how new GMT modules should be wrapped
Easiest way would be to modify the 'Reminders' in the Pull Request template at https://github.com/GenericMappingTools/pygmt/blame/v0.5.0/.github/PULL_REQUEST_TEMPLATE.md#L11-L17. Specifically, the - [ ] If adding new functionality, add an example to docstrings or tutorials.
line should be made optional or something.
Other than that, we could put a note in https://github.com/GenericMappingTools/pygmt/blob/v0.5.0/doc/contributing.md that these are the general stages on wrapping a GMT module:
- Wishlist
- 'Wrapper for XXX' feature request issue
- 'Wrap XXX' initial feature implementation PR
- 'Add missing aliases to XXX' documentation PR / 'Support some functionality in XXX' enhancement PR
- 'Add gallery example for XXX' documentation PR
- 'Add tutorial for XXX' documentation PR (optional)
Note that step 3 and 4 can be done in parallel in some cases.
Tracking the status (feature completeness) of a PyGMT module
Ideally we would have an issue to track a PyGMT module from its initial implementation to the final gallery example/tutorial, but that's a bit of a hassle. So let's track it using cards instead. See https://github.com/GenericMappingTools/pygmt/projects/9
If any of you have a new wishlist item, or find an incomplete module that's not added, please add it to the project board!
Originally posted by @weiji14 in #1072 (comment)
Just on this point (and as I've mentioned to Jiayuan at #1145 (review)), I'd recommend wrapping as few aliases as you can get away with in this PR. We can open up separate PRs to wrap any complicated parameter like
inquire=I
, ortransparency=t
, and the gallery example/tutorial can be left for another day too.To be clear, that means you just have to alias the required arguments (https://docs.generic-mapping-tools.org/dev/histogram.html#required-arguments), and the common aliases (-R, -J, -c, -p, -t, etc) to pass. Maybe add one minimal unit test to hit the code coverage target. I see you've done a few aliases already (and you can leave them in if you want), but don't feel like you have to complete all of them.
Example workflow would be to do: Minimal PR (this one) -> Gallery Example PR -> Complete documentation/aliases PR -> Full Tutorial PR
P.S. Yes, we should document this 'policy' about wrapping modules chunk by chunk somewhere, but thought I'd mention it to you first.
Are you willing to help implement and maintain this feature? Yes/No