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

Reorganize reference docs #1753

Merged
merged 9 commits into from Jun 29, 2021
Merged

Reorganize reference docs #1753

merged 9 commits into from Jun 29, 2021

Conversation

ivirshup
Copy link
Member

@ivirshup ivirshup commented Mar 21, 2021

This PR contains a reorganization of the API reference docs, mostly to ease modification and generation.

Fixes #1682

Case sensitivity

First big issue addressed, scanpy.plotting.dotplot.rst and scanpy.plotting.DotPlot.rst are the same path on a case insensitive file system. This is what most people on macOS will have. If you have a case sensitive file system, important apps like Adobe's and steam won't work. So now the plotting classes are put in generated/classes.

For me, this means:

  • docs now generate without warnings
  • DotPlot and MatrixPlot docs are only rebuilt if I update them

Separate tracked and generated content

I find it difficult to navigate the rst api docs when there is a large amount of auto generated files mixed in with manually curated ones. This becomes more of a problem since the autogenerated files are not removed by make clean and will still generate html files and generally be a nuisance while they are still around.

Now all autogenerated API files are put in a generated directory. This is also what xarray, scikit-learn, and seaborn do.

Now make clean also deletes all auto generated rst files. This also dramatically simplifies our .gitignore

Consolidation

I've also consolidated the managed api docs to a smaller number of files. Namely, instead of api/*.rst there's now just api.rst. Instead of external/*.rst there's just external.rst.

Plotting functions API docs

In the current api docs, some plotting functions have a plot displayed inline instead of a one line description. This is nice because it provides a visual reference for what the plotting function does. It's less nice because:

  • It takes up a huge amount of space
  • It takes up a different amount of space for each function
  • These plots are manually generated, and are often quite out of date
example image

I think the reference docs should be handled in the same way for each of the modules. Example plots make more sense in the user guide/ tutorials and within each functions documentation (#1664).

I'd like to go further with this by moving the plotting tutorial's content to the user guide, so we can have more in depth linking to different sections. Additional benefits here include plots being generated with each doc build and having links from functions in the guide to their api docs.

TODO

  • Redirects (redirects need to be added on readthedocs so old links won't fail) added
  • Decide where module docs live, as doc-string, or as rst. rst

* Save generated api docs to their own directory
* Seperate generated classes and functions for case insensitive file systems
* Disabled images in signatures for plotting functions
@codecov
Copy link

codecov bot commented Mar 21, 2021

Codecov Report

Merging #1753 (deb2d91) into master (d3e6e42) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1753      +/-   ##
==========================================
- Coverage   71.61%   71.60%   -0.01%     
==========================================
  Files          92       92              
  Lines       11252    11250       -2     
==========================================
- Hits         8058     8056       -2     
  Misses       3194     3194              
Impacted Files Coverage Δ
scanpy/external/__init__.py 100.00% <ø> (ø)
scanpy/plotting/__init__.py 100.00% <ø> (ø)

@ivirshup ivirshup marked this pull request as ready for review March 23, 2021 01:28
@ivirshup
Copy link
Member Author

@flying-sheep, any suggestions on how to automatically clear out all the rst files from api and extension? I suppose we could add a temporary git clean, but that seems a bit harsh.

docs/conf.py Outdated
Comment on lines 54 to 60
*[p.stem for p in (HERE / 'extensions').glob('*.py')],
# Locally defined
"debug_docstrings",
"github_links",
"param_police",
"typed_returns",
# "function_images",
# *[p.stem for p in (HERE / 'extensions').glob('*.py')],
Copy link
Member

Choose a reason for hiding this comment

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

Yes, this should be generated with some plotting gallery plugin instead.

Let’s move this part to the PR that adds this plugin.

Copy link
Member Author

Choose a reason for hiding this comment

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

As mentioned in the main PR comment, I don't think there should be example plots in the table of contents for the reference docs. Examples fit in tutorials, the docs for the function, and examples, but not the table of contents.

Copy link
Member

@flying-sheep flying-sheep left a comment

Choose a reason for hiding this comment

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

Regarding generated, there’s both no need and actual downsides:

  1. git clean can be used with a prefix path: git clean -fx -- ./docs

  2. Changing anything wide-reaching this would break many of our incoming links: Anyone who has ever linked any autogenerated doc page would now have a broken link. Not a fan of that part …

  3. For case insensitivity let’s use the feature that specifically exists for that problem instead: autosummary_filename_map

    A dict mapping object names to filenames. This is necessary to avoid filename conflicts where multiple objects have names that are indistinguishable when case is ignored, on file systems where filenames are case-insensitive.

This way you can still build the docs without problems, with the only downside being that links to two objects break.

@ivirshup
Copy link
Member Author

ivirshup commented Apr 6, 2021

Regarding generated, there’s both no need

I disagree with this. I think we definitely should not be intermingling source and generated files, especially when it's one source file to many generated. This is not a pleasant way to navigate files:

image

Also, looking around at other big projects with sphinx documentation, this is the way they all handled autosummary stub files.

git clean -fx -- ./docs

This is a bit of a nuke. I've added removing the generated docs to make clean, but we wouldn't want to run git clean -fx -- ./docs for make clean.

  1. Changing anything wide-reaching this would break many of our incoming links

This would be solved with redirects.

I would be happy to have docs.scanpy.org/en/latest/api/dotplot.html be the canonical url, but not if it requires mixing generated and source files.

  1. For case insensitivity let’s use the feature that specifically exists for that problem instead: autosummary_filename_map

Doesn't this only half solve the issue? Since (AFAIK) the generated html page has to have the same name as the rst file, this would break links (as you mentioned above). While we could do a redirect from here, what do we redirect too? Do we have special names for urls for classes which have an associated function? I don't think that would be cleaner than just having all classes be put in a classes subdirectory.

@flying-sheep
Copy link
Member

flying-sheep commented Apr 6, 2021

This is not a pleasant way to navigate files:

You can fix that with the .ignore plugin!

Right click the Project view and select “Hide Ignored Files”

grafik

I would be happy to have docs.scanpy.org/en/latest/api/dotplot.html be the canonical url, but not if it requires mixing generated and source files.

That would be perfect if it’s possible!

Since (AFAIK) the generated html page has to have the same name as the rst file, this would break links (as you mentioned above). While we could do a redirect from here, what do we redirect too?

We create manual redirects for the 2 APIs where we failed to add an underscore to the function name, and don’t do that again.

The non-domain parts of URLs are case sensitive, therefore having a redirect .../dotplot.html.../dot_plot.html works perfectly fine in the presence of .../DotPlot.html

@ivirshup
Copy link
Member Author

I'm going to merge this since it makes my life much easier.

We can always add more redirects if we want to put the reference docs somewhere else in the future.

@ivirshup ivirshup merged commit 7791e75 into scverse:master Jun 29, 2021
@ivirshup ivirshup deleted the reorg-docs branch June 29, 2021 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plotting functions should show up in table of contents
2 participants