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

Improve PyGMT documentation #996

Closed
wants to merge 37 commits into from

Conversation

core-man
Copy link
Member

@core-man core-man commented Mar 3, 2021

Description of proposed changes

Related to #983.

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

@core-man core-man marked this pull request as draft March 3, 2021 07:19
@core-man core-man changed the title Improve installation doc WIP: Improve installation doc Mar 3, 2021
doc/install.rst Outdated Show resolved Hide resolved
@core-man core-man changed the title WIP: Improve installation doc WIP: Improve PyGMT documentation Mar 3, 2021
@core-man
Copy link
Member Author

core-man commented Mar 3, 2021

@GenericMappingTools/python Is it good to submit revisions for different .rst files in this single PR? Now I made some revisions for install.rst and first-figure.rst. But based on changelog, I found you update each documentation in a single PR.

@willschlitzer
Copy link
Contributor

@GenericMappingTools/python Is it good to submit revisions for different .rst files in this single PR? Now I made some revisions for install.rst and first-figure.rst. But based on changelog, I found you update each documentation in a single PR.

It's situation dependent and a matter of opinion. In my opinion, if you're making big changes to the files, probably best to split it into multiple PRs so all of the file changes aren't dependent on a single PR getting approved. But if it's relatively minor changes (formatting, spelling, adding a single line/description to multiple files) I think they should be grouped together in a single PR.

doc/install.rst Outdated Show resolved Hide resolved
doc/install.rst Outdated Show resolved Hide resolved
doc/install.rst Outdated Show resolved Hide resolved
doc/install.rst Outdated Show resolved Hide resolved
doc/install.rst Outdated Show resolved Hide resolved
examples/tutorials/first-figure.py Outdated Show resolved Hide resolved
examples/tutorials/first-figure.py Outdated Show resolved Hide resolved
examples/tutorials/first-figure.py Show resolved Hide resolved
@seisman seisman added the documentation Improvements or additions to documentation label Mar 3, 2021
@seisman seisman added this to the 0.3.1 milestone Mar 3, 2021
Co-authored-by: Dongdong Tian <[email protected]>
Co-authored-by: Meghan Jones <[email protected]>
doc/install.rst Outdated Show resolved Hide resolved
@core-man core-man changed the title WIP: Improve PyGMT documentation Improve PyGMT documentation Mar 6, 2021
@core-man core-man marked this pull request as ready for review March 6, 2021 13:21
doc/install.rst Outdated Show resolved Hide resolved
examples/gallery/line/linestyles.py Outdated Show resolved Hide resolved
@weiji14
Copy link
Member

weiji14 commented Mar 7, 2021

@GenericMappingTools/python Is it good to submit revisions for different .rst files in this single PR? Now I made some revisions for install.rst and first-figure.rst. But based on changelog, I found you update each documentation in a single PR.

It's situation dependent and a matter of opinion. In my opinion, if you're making big changes to the files, probably best to split it into multiple PRs so all of the file changes aren't dependent on a single PR getting approved. But if it's relatively minor changes (formatting, spelling, adding a single line/description to multiple files) I think they should be grouped together in a single PR.

@core-man, I appreciate all the improvements you're making here, but this Pull Request is getting a bit large in scope. The general guidelines at https://github.com/GenericMappingTools/pygmt/blob/v0.3.0/CONTRIBUTING.md#general-guidelines recommend that:

  • Each pull request should consist of a small and logical collection of changes.
  • Larger changes should be broken down into smaller components and integrated separately.

In particular, the changes made to the gallery examples might cause some tricky merge conflicts with the gallery reorganization at #995. I suggest you coordinate a bit with @willschlitzer on this matter (i.e. decide who merges their PR first) before things get more complicated.

@core-man
Copy link
Member Author

core-man commented Mar 7, 2021

@core-man, I appreciate all the improvements you're making here, but this Pull Request is getting a bit large in scope. The general guidelines at https://github.com/GenericMappingTools/pygmt/blob/v0.3.0/CONTRIBUTING.md#general-guidelines recommend that:

  • Each pull request should consist of a small and logical collection of changes.
  • Larger changes should be broken down into smaller components and integrated separately.

Thanks for the suggestion. I agree this PR is a bit large in scope, although I
plan to separate some other revisions (e.g., #983 (comment), #983 (comment)) in other PRs.

In particular, the changes made to the gallery examples might cause some tricky merge conflicts with the gallery reorganization at #995. I suggest you coordinate a bit with @willschlitzer on this matter (i.e. decide who merges their PR first) before things get more complicated.

yes, I also notice #995 which I think should be merged first.

@core-man
Copy link
Member Author

core-man commented Mar 9, 2021

Based on the suggestion by @weiji14, I will re-submit all the revisions in this PR by a few small PRs. Each small PR only contains revisions of 1~2 files. After all the revisions are re-submitted, I will close this PR. #1025 is the first one. Sorry for the inconvenience.

@seisman
Copy link
Member

seisman commented Mar 13, 2021

@core-man Do you want to apply some changes in this PR before v0.3.1? If you do, please open separate PRs for them.

@core-man
Copy link
Member Author

core-man commented Mar 13, 2021

@core-man Do you want to apply some changes in this PR before v0.3.1? If you do, please open separate PRs for them.

Okay.

@core-man
Copy link
Member Author

Close this PR since the revisions have been submitted by the individual PRs.

@core-man core-man closed this Mar 13, 2021
@core-man core-man deleted the imporve-doc branch March 14, 2021 09:08
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.

7 participants