pygmt icon indicating copy to clipboard operation
pygmt copied to clipboard

Policy on wrapping new GMT modules step by step from initial implementation to gallery example

Open weiji14 opened this issue 4 years ago • 14 comments
trafficstars

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:

  1. Wishlist
  2. 'Wrapper for XXX' feature request issue
  3. 'Wrap XXX' initial feature implementation PR
  4. 'Add missing aliases to XXX' documentation PR / 'Support some functionality in XXX' enhancement PR
  5. 'Add gallery example for XXX' documentation PR
  6. '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

image

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 https://github.com/GenericMappingTools/pygmt/issues/1072#issuecomment-815292540

Just on this point (and as I've mentioned to Jiayuan at https://github.com/GenericMappingTools/pygmt/pull/1145#pullrequestreview-628868487), 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, or transparency=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

weiji14 avatar Oct 31 '21 23:10 weiji14

Thanks for the great write-up! I agree with both points about issues being ideal for tracking and also a bit of a hassle. Maybe it's just that I am not used to it yet, but I find it a bit difficult to determine where on the path to completeness a module is based on the project board. What do you think about having a separate Feature request: wrap new GMT module issue template that includes a checklist for these steps? This could be in addition to your points about modifying the contributing guide and pull request template.

maxrjones avatar Nov 01 '21 19:11 maxrjones

Thanks for the great write-up! I agree with both points about issues being ideal for tracking and also a bit of a hassle. Maybe it's just that I am not used to it yet, but I find it a bit difficult to determine where on the path to completeness a module is based on the project board. What do you think about having a separate Feature request: wrap new GMT module issue template that includes a checklist for these steps? This could be in addition to your points about modifying the contributing guide and pull request template.

I think this is a good idea. It's easy to say we want to wrap something in small, manageable chunks, but that's not specific and someone new to the project (or not new at all) may be confused on what the expectations are for wrapping a function.

willschlitzer avatar Nov 02 '21 07:11 willschlitzer

@weiji14 Are you envisioning this as an addition to CONTRIBUTING.md or should it be somewhere else in the documentation?

willschlitzer avatar Nov 19 '21 07:11 willschlitzer

@weiji14 Are you envisioning this as an addition to CONTRIBUTING.md or should it be somewhere else in the documentation?

Yes, I think it should be in CONTRIBUTING.md somewhere, but the Pull Request template's checklist also needs some modification.

weiji14 avatar Nov 20 '21 07:11 weiji14

I'm starting to get a bit more used to the project board and think this could be really helpful. I have a couple suggestions:

  1. Include a checklist on the cards that gives context to the referenced issues/PRs(e.g., https://github.com/GenericMappingTools/pygmt/projects/9#card-71936074) This might be redundant with the feature request issues, so it could be only applied to the modules that aren't being tracked by an issue.
  2. Combine panels 4 (gallery) and 5 (tutorial) since not all modules will need both.

maxrjones avatar Dec 14 '21 22:12 maxrjones

I'm a fan of the project board as well; I think it makes it easier to look at the big picture and see what tasks need to be done, rather than look to find issues for specific modules to see if they need additional features.

This might be redundant with the feature request issues, so it could be only applied to the modules that aren't being tracked by an issue.

I don't think we have many new modules being tracked by an issue. I think it would be easiest to transition existing feature request issues to the project board, rather than leave issues open for potentially a long time while we wait for the later stages of a module getting wrapped.

  1. Combine panels 4 (gallery) and 5 (tutorial) since not all modules will need both.

Agreed.

willschlitzer avatar Dec 15 '21 07:12 willschlitzer

What do you think about modifying the project board so that the modules are placed in the column that requires completion rather than the column that has been completed? I think this would be more clear for finding out what needs to be done and would better account for the fact that not all steps are always completed in order. For example, wiggle is currently in the column 'Added gallery|tutorial example', which is true but not that helpful because it still has missing aliases.

My suggested columns are:

  1. Feature request issue (where module cards live when there's only an issue open for discussion)
  2. Initial implementation (modules that are being implemented for the first time)
  3. Complete aliases (modules that have an initial implementation but are missing aliases, which may be in progress)
  4. Gallery|tutorial example (modules that need a gallery or tutorial, which may be in progress)
  5. Ongoing maintenance (modules that have steps 1-4 completed)

maxrjones avatar Mar 29 '22 21:03 maxrjones

Sounds good to me. Should we start to use the Projects (beta) instead?

seisman avatar Mar 30 '22 03:03 seisman

I think it is a good idea for modules to be in the column with work to be done instead of work last completed.

I'm not opposed to Projects (beta), but looking at the GitHub page for it, I'm not sure what it will enable us to do that isn't already covered on the current module wrapping project.

willschlitzer avatar Mar 30 '22 06:03 willschlitzer

I don't know of a way to transfer projects from Projects to Projects (beta) anyways. Maybe they will add this feature when it's out of beta. For now, I think we could start try out Projects (beta) for new projects.

maxrjones avatar Mar 30 '22 15:03 maxrjones