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

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

Open
weiji14 opened this issue Oct 31, 2021 · 10 comments · May be fixed by #1687
Open

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

weiji14 opened this issue Oct 31, 2021 · 10 comments · May be fixed by #1687
Labels
documentation Improvements or additions to documentation

Comments

@weiji14
Copy link
Member

weiji14 commented Oct 31, 2021

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 #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, 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 weiji14 added the documentation Improvements or additions to documentation label Oct 31, 2021
@weiji14 weiji14 added this to the 0.6.0 milestone Oct 31, 2021
@maxrjones
Copy link
Member

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.

@willschlitzer
Copy link
Contributor

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
Copy link
Contributor

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

@weiji14
Copy link
Member Author

weiji14 commented Nov 20, 2021

@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.

@maxrjones
Copy link
Member

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.

@willschlitzer
Copy link
Contributor

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.

@maxrjones maxrjones linked a pull request Dec 27, 2021 that will close this issue
6 tasks
@weiji14 weiji14 modified the milestones: 0.6.0, 0.6.1 Mar 11, 2022
@maxrjones
Copy link
Member

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)

@seisman
Copy link
Member

seisman commented Mar 30, 2022

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

@willschlitzer
Copy link
Contributor

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.

@maxrjones
Copy link
Member

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.

@seisman seisman modified the milestones: 0.6.1, 0.7.0 Apr 9, 2022
@seisman seisman modified the milestones: 0.7.0, 0.8.0 Jun 23, 2022
@seisman seisman modified the milestones: 0.8.0, 0.9.0 Dec 11, 2022
@seisman seisman moved this to 0) Wishlist in PyGMT: Wrapping GMT modules Mar 17, 2023
@seisman seisman modified the milestones: 0.9.0, 0.10.0 Mar 19, 2023
@weiji14 weiji14 modified the milestones: 0.10.0, 0.11.0 Aug 24, 2023
@seisman seisman modified the milestones: 0.11.0, 0.12.0 Dec 11, 2023
@seisman seisman removed this from the 0.12.0 milestone Feb 26, 2024
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
Projects
Status: 0) Wishlist
Development

Successfully merging a pull request may close this issue.

4 participants