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

How to reproduce original GMT arguments in PyGMT documentation #631

Closed
weiji14 opened this issue Sep 23, 2020 · 25 comments · Fixed by #1182
Closed

How to reproduce original GMT arguments in PyGMT documentation #631

weiji14 opened this issue Sep 23, 2020 · 25 comments · Fixed by #1182
Assignees
Labels
documentation Improvements or additions to documentation help wanted Helping hands are appreciated maintenance Boring but important stuff for the core devs
Milestone

Comments

@weiji14
Copy link
Member

weiji14 commented Sep 23, 2020

Description of the desired feature

The GMT documentation has many instances where bold and italicized fonts are mixed, looking something like:

+nargs

The PyGMT documentation has been rather inconsistent with this (see mentions at #525 (comment) and #620 (comment)).

What we should do is to standardize how we want +n args to be written in the documentation. This is a bit complicated, but boils down to (at least) two options:

  1. r" **+n**\ *args* ". Use a raw-string in Python, allowing us to follow upstream GMT (as per Complete documentation of grdimage #620 (comment))

  2. " **+n**\\ *args* ". Use a double-backslash to avoid the flake8 error W605 invalid escape sequence '\ '.

An alternative would be to find a way to ignore the flake8 error somehow for the ReST docstrings.

Also, should we find a way to put these in code-formatted blocks (i.e. with backticks), as mentioned at https://github.com/GenericMappingTools/pygmt/pull/525/files#r454746311?

Are you willing to help implement and maintain this feature? Somewhat short on time, but will look into it.

References:

@weiji14 weiji14 added the documentation Improvements or additions to documentation label Sep 23, 2020
@seisman
Copy link
Member

seisman commented Dec 17, 2020

This is what PEP-257 says:

For consistency, always use """triple double quotes""" around docstrings. Use r"""raw triple double quotes""" if you use any backslashes in your docstrings. For Unicode docstrings, use u"""Unicode triple-quoted strings""".

So, instead of using r" **+n**\ *args* " for each individual cases, we should use r for the whole docstring, e.g.,

def function():
    r"""
    A long docstring string.

    We now can write doctsrings with backslashes **+n**\ *args*
    """

@seisman
Copy link
Member

seisman commented Dec 18, 2020

As pointed out by @weiji14, the PyGMT documentation is inconsistently using italics, bold, and code strings.

I think we can following these conventions:

  1. Use code for Python function parameters
  2. Use bold and italics for GMT syntax like +nargs, in which bold +n means it should be typed as it is, and italic args should be given a specific value.

This is a good example of what it looks like (https://www.pygmt.org/dev/tutorials/frames.html):

image

@seisman
Copy link
Member

seisman commented Dec 18, 2020

Ping @GenericMappingTools/python @willschlitzer for thoughts on this.

@willschlitzer
Copy link
Contributor

@seisman I like this convention for explaining literal titles and the associated arguments that go with them. Are you thinking there should be similar formatting in the string of arguments given in the doctstring?

@seisman
Copy link
Member

seisman commented Dec 19, 2020

Are you thinking there should be similar formatting in the string of arguments given in the doctstring?

Yes, but need feedbacks first.

@seisman
Copy link
Member

seisman commented Dec 22, 2020

Ping @GenericMappingTools/python for comments.

@seisman
Copy link
Member

seisman commented Dec 29, 2020

Ping @GenericMappingTools/python, @GenericMappingTools/python-maintainers and @GenericMappingTools/python-contributors for thoughts on #631 (comment) and #631 (comment).

@weiji14
Copy link
Member Author

weiji14 commented Dec 29, 2020

Let's go with Option 1 (using the raw string). Easier to copy from GMT, and avoids messy backslashes. Also agree on the backtick code blocks.

Will need to document this (with an example) somewhere in CONTRIBUTING.md

@willschlitzer
Copy link
Contributor

I think making the entire docstring a raw string is the way to go; I think a double backslash might be confusing to some, and copying directly from GMT makes life easier.

I can write this up in CONTRIBUTING.md when we come to a consensus.

@seisman
Copy link
Member

seisman commented Dec 29, 2020

I can write this up in CONTRIBUTING.md when we come to a consensus.

At least three of the maintainers agree with this. I think we can do it now.

@seisman
Copy link
Member

seisman commented Dec 30, 2020

As most of us agree with the new docstring conventions, we should do:

  • document the docstring conventions
  • review the whole project and fix any inconsistency

@seisman seisman added this to the 0.3.0 milestone Dec 30, 2020
@seisman seisman added good first issue Good for newcomers help wanted Helping hands are appreciated maintenance Boring but important stuff for the core devs labels Dec 30, 2020
@willschlitzer
Copy link
Contributor

willschlitzer commented Dec 30, 2020

When referencing and explaining the written code in documentation, should it all be in code format or should it the bold/italicize documentation format. Some examples I encountered in earth-relief.py:

"a2500" or a2500
"x+lElevation" or x+lElevation

My opinion is to leave it all as code formatting to denote that it is the literal string being passed, but I can see it both ways.

@seisman
Copy link
Member

seisman commented Dec 30, 2020

My opinion is to leave it all as code formatting to denote that it is the literal string being passed

I think this is the better way.

@weiji14
Copy link
Member Author

weiji14 commented Jan 21, 2021

I've just been looking through some of the new API docs and noticed a problem. E.g. https://www.pygmt.org/dev/api/generated/pygmt.Figure.coast.html after #765 looks like this:

image

I think we need to add a fullstop/period (.) between [+z] and Select ... to break the flow, or have Select ... appear on a newline as in the GMT docs:

image

@weiji14
Copy link
Member Author

weiji14 commented Feb 12, 2021

Anything else remaining after #862?

@seisman
Copy link
Member

seisman commented Feb 12, 2021

Some module-specific arguments still need updates, for example, basemap's map_scale argument.

@maxrjones
Copy link
Member

Some module-specific arguments still need updates, for example, basemap's map_scale argument.

Let me know if you'd like me to help with updating these.

@willschlitzer
Copy link
Contributor

@meghanrjones Please do! I'm planning on going through the documentation tomorrow; no complaints if you submit a PR for changes!

@maxrjones
Copy link
Member

@meghanrjones Please do! I'm planning on going through the documentation tomorrow; no complaints if you submit a PR for changes!

Sounds good. I'll try to get it done before then.

@seisman
Copy link
Member

seisman commented Feb 12, 2021

@meghanrjones Please do! I'm planning on going through the documentation tomorrow; no complaints if you submit a PR for changes!

Sounds good. I'll try to get it done before then.

Great! Please try your best to follow the standards as documented here (https://github.com/GenericMappingTools/pygmt/blob/master/CONTRIBUTING.md#example-code-standards).

There may be some cases that are not covered by the standards. We can discuss more later.

@seisman seisman modified the milestones: 0.3.0, 0.4.0 Feb 14, 2021
weiji14 added a commit that referenced this issue Feb 14, 2021
Update docstrings in basemap, coast, colorbar, contour,
grdcontour, grdimage, grdview, image, inset, legend, logo,
plot, plot3d, and text following the convention in #631.
Also fixed an unescaped return in decorators.py.

Co-authored-by: Dongdong Tian <[email protected]>
Co-authored-by: Will Schlitzer <[email protected]>
Co-authored-by: Wei Ji <[email protected]>
@maxrjones
Copy link
Member

maxrjones commented Feb 19, 2021

Is the convention for documenting defaults in PyGMT set? In GMT, it is typically brackets [ ] without any explanation before the default. I remember a comment in which @seisman mentioned that "[Default is ...]" is more clear than just "[...]", but I couldn't find that comment again.

@seisman
Copy link
Member

seisman commented Feb 19, 2021

In GMT, it is typically brackets [ ] without any explanation before the default. I remember a comment in which @seisman mentioned that "[Default is ...]" is more clear than just "[...]",

Yes, it's open for discussions. The GMT documentation sometimes also uses "[Default is ...]".

@maxrjones
Copy link
Member

Moving forward is seems that the GMT documentation will be going with "[Default is ...]" (see GenericMappingTools/gmt#4834).

seisman added a commit that referenced this issue Feb 22, 2021
This PR adds links to xarray, pandas, and numpy classes in the Returns 
section of the docstrings. It also includes a few small fixes according 
to the conventions set in #631.

Co-authored-by: Dongdong Tian <[email protected]>
@seisman
Copy link
Member

seisman commented Mar 27, 2021

Moving forward is seems that the GMT documentation will be going with "[Default is ...]" (see GenericMappingTools/gmt#4834).

I think we can document it in the "Example code standards" section, then close the issue, right?

@maxrjones
Copy link
Member

maxrjones commented Mar 29, 2021

Moving forward is seems that the GMT documentation will be going with "[Default is ...]" (see GenericMappingTools/gmt#4834).

I think we can document it in the "Example code standards" section, then close the issue, right?

Yes, I can add that guideline after #1126 is merged.

@maxrjones maxrjones self-assigned this Mar 29, 2021
@michaelgrund michaelgrund mentioned this issue Mar 31, 2021
5 tasks
@seisman seisman removed the good first issue Good for newcomers label Apr 2, 2021
sixy6e pushed a commit to sixy6e/pygmt that referenced this issue Dec 21, 2022
Update docstrings in basemap, coast, colorbar, contour,
grdcontour, grdimage, grdview, image, inset, legend, logo,
plot, plot3d, and text following the convention in GenericMappingTools#631.
Also fixed an unescaped return in decorators.py.

Co-authored-by: Dongdong Tian <[email protected]>
Co-authored-by: Will Schlitzer <[email protected]>
Co-authored-by: Wei Ji <[email protected]>
sixy6e pushed a commit to sixy6e/pygmt that referenced this issue Dec 21, 2022
This PR adds links to xarray, pandas, and numpy classes in the Returns 
section of the docstrings. It also includes a few small fixes according 
to the conventions set in GenericMappingTools#631.

Co-authored-by: Dongdong Tian <[email protected]>
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 help wanted Helping hands are appreciated maintenance Boring but important stuff for the core devs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants