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

Error in contour documentation (-A argument) #879

Closed
maxrjones opened this issue Feb 13, 2021 · 8 comments · Fixed by #883
Closed

Error in contour documentation (-A argument) #879

maxrjones opened this issue Feb 13, 2021 · 8 comments · Fixed by #883
Assignees
Labels
bug Something isn't working documentation Improvements or additions to documentation

Comments

@maxrjones
Copy link
Member

Description of the problem

This description of the A option for pygmt.Figure.contour in the PyGMT docs is incorrect.

PyGMT description:

A (bool or str) – '[m|p|x|y]' By default, geographic line segments are drawn as great circle arcs. To draw them as straight lines, use A.

GMT description:

-A[n|contours][labelinfo] contours is annotation interval in data units; it is ignored if contour levels are given in a file via -C. [Default is no annotations]. Prepend n to disable all annotations implied by -C. To just select a few specific contours give them as a comma-separated string; if only a single contour please add a trailing comma so it is seen as a list and not a contour interval. The optional labelinfo controls the specifics of the label formatting and consists of a concatenated string made up of any of the following control arguments:

Since A argument is the same for contour and grdcontour, I propose to add the same alias as in grdcontour for A (annotation).

@maxrjones maxrjones added bug Something isn't working documentation Improvements or additions to documentation labels Feb 13, 2021
@welcome
Copy link

welcome bot commented Feb 13, 2021

👋 Thanks for opening your first issue here! Please make sure you filled out the template with as much detail as possible. You might also want to take a look at our contributing guidelines and code of conduct.

@maxrjones
Copy link
Member Author

If the alias addition is alright, I can fix this issue.

It also might be a good time to clean "[TODO: Insert more documentation]" in the contour docs before v0.3.0. While the argument descriptions can be cleaned up a bit, I actually don't think that anything more is really needed in the preamble since there's a link to the GMT docs and the amount of information provided is on par with most of the other methods.

@weiji14
Copy link
Member

weiji14 commented Feb 13, 2021

Hi @meghanrjones, annotation for -A sounds good. Go for it!

Also, feel free to suggest other aliases for the arguments in contour. We're trying to move away from single character arguments (edit: see #262 and #473).

@maxrjones maxrjones self-assigned this Feb 13, 2021
@maxrjones
Copy link
Member Author

Great. One thing that I do not understand from the contributing guidelines - how do you decide which argument flags go into the PyGMT docstrings? For example, the pygmt docs for grdcontour for 'annotation' has a subset of the many flags for gmt's grdcontour -A.

@weiji14
Copy link
Member

weiji14 commented Feb 13, 2021

The goal is to wrap all of them if possible. But historically, we were lazy and only aliased a subset, usually the required arguments, or those that were subjectively deemed as important. It's better to start with something than nothing I guess, as we can always iterate and add more later. Some modules in GMT have dozens of arguments and it can take a lot of time to review them all!

@maxrjones
Copy link
Member Author

So, if I understand correctly, the subset of the flags that are included in the documentation are the ones that have been tested? Perhaps a better example of my confusion is basemap. map_scale and rose take really similar option flags and map_scale lists a subset of the flags from basemap -L whereas rose does not list any flags from basemap -T. Still, map_scale does not list all the option flags from basemap -L and likely wouldn't work with only '[g|j|J|n|x]refpoint'.

@weiji14
Copy link
Member

weiji14 commented Feb 13, 2021

So, if I understand correctly, the subset of the flags that are included in the documentation are the ones that have been tested?

Yes and no, it depends on many factors. On what should or should not be tested, see #796 and https://github.com/GenericMappingTools/pygmt/blame/a828f739057f6fd003d131063eab5e04df265540/CONTRIBUTING.md#L321-L325. Let's just say our standards have changed since PyGMT started in 2017, and there are some parts of our documentation from that early era (2018-2019) that have not been updated to the new standard (2021)! Which is why we need more eyes and contributors working on this!

Perhaps a better example of my confusion is basemap. map_scale and rose take really similar option flags and map_scale lists a subset of the flags from basemap -L whereas rose does not list any flags from basemap -T. Still, map_scale does not list all the option flags from basemap -L and likely wouldn't work with only '[g|j|J|n|x]refpoint'.

Fair point. the basemap is certainly a priority to document properly since it is one of the more commonly used functions. We had some PRs dedicated to completing the documentation of some functions (e.g. #799, #676, etc), I'd encourage you to take a look at those PRs to see how wrapping new aliases can be done.

@maxrjones
Copy link
Member Author

Thanks for the explanation and the links to the PRs! Those will be helpful to review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants