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

Fix information cut off in crs docs #2183

Merged
merged 4 commits into from
Jun 7, 2023
Merged

Conversation

lgolston
Copy link
Contributor

@lgolston lgolston commented Jun 5, 2023

Rationale

In the core CRS documentation page (https://scitools.org.uk/cartopy/docs/latest/reference/generated/cartopy.crs.CRS.html#), there is rather limited information shown since a majority of the notes are simply “New in version 2.2.0.” or in a few cases cut off, e.g. “Make a CRS from:”.

Also, in the mpl docs pages a description of the input parameters are missing in the current 0.21.0 version (see e.g. https://scitools.org.uk/cartopy/docs/latest/reference/generated/cartopy.mpl.gridliner.Gridliner.html).

Implementation

This is addressed in this PR by creating a new Sphinx template class_with_inherited.rst and directing autosummary to use it in the reference/crs.rst doc page, causing the full docstring to be shown rather than just the first line. To reduce the size of the generated pages for classes that inherit CRS, they are set to use class_without_inherited, which is already being used by reference/matplotlib.rst. That template was modified to show init, addressing the missing parameter details.

Implications

The information identified in cartopy.crs.CRS and cartopy.mpl docs that was missing is now shown

Copy link
Contributor

@greglucas greglucas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this looks better to not have the pyproj version information within Cartopy.

@rcomer, you've been doing a lot of documentation review on Matplotlib, do you have any comments on this or know why it isn't showing the CircleCI redirect anymore?

@QuLogic
Copy link
Member

QuLogic commented Jun 6, 2023

@rcomer, you've been doing a lot of documentation review on Matplotlib, do you have any comments on this or know why it isn't showing the CircleCI redirect anymore?

We need to add a token, as was done in Matplotlib matplotlib/matplotlib#25672 I was hoping someone from the Met Office would do that

@rcomer
Copy link
Member

rcomer commented Jun 6, 2023

I'm afraid I don't know much at all about sphinx directives. @tkknight is our local documentation expert so perhaps he has thoughts.

For the redirect issue, I believe @bjlittle is now on the case.

@bjlittle
Copy link
Member

bjlittle commented Jun 6, 2023

We need to add a token, as was done in Matplotlib matplotlib/matplotlib#25672 I was hoping someone from the Met Office would do that

See #2185.

HTH

@lgolston
Copy link
Contributor Author

lgolston commented Jun 6, 2023

I checked the way matplotlib docs are configured and see that it is cleaner to add autoclass_content = 'both' to the conf.py file rather than :special-members: init in the template. This is now changed.

Here is some links representative of the modified docs:
CRS doc Live ver - PR ver

Projection doc Live ver - PR ver

mp.gridliner doc Live ver - PR ver

@greglucas
Copy link
Contributor

Nice! I think this is an improvement. Do you want the inherited members of CRS? I find the ..version callouts for pyproj versions to be confusing here because I would think it would relate to Cartopy's version numbers.
The other two pages are definitely better this way IMO.

@lgolston
Copy link
Contributor Author

lgolston commented Jun 7, 2023

Thanks! Initially I liked showing the inherited methods since that library is not as common/familiar as something like matplotlib. However, I agree the version callouts are a little confusing and don't like emphasizing the various inherited from_ methods since those don't even work in cartopy. Am happy to go ahead with not showing the inherited members here, especially with it now linking to pyproj.crs.CRS at the top of the page.

@greglucas greglucas added this to the 0.22 milestone Jun 7, 2023
@greglucas
Copy link
Contributor

Looks good to me. Thank you @lgolston!

@greglucas greglucas merged commit 8528efe into SciTools:main Jun 7, 2023
@lgolston lgolston deleted the sphinx-crs branch June 7, 2023 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants