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

Move the backend docs and connect the config docs. Both in a single sidebar entry. #7389

Merged
merged 5 commits into from Apr 5, 2024

Conversation

dschult
Copy link
Member

@dschult dschult commented Apr 4, 2024

Here's one attempt at separating the backend docs from other utilities. It also adds the config docs to the html documentation on the same page as the backend description. When other config options (non backend) get introduced we should separate the config and backend pages.

So, now in the Reference pages there is a sidebar entry called "Backends and Configs". It describes the backend system, the _dispatchable decorator, and the nx.config object.

It'd be nice to have an overview of the nx.config object and its purpose and how to use it. But this PR only moves what we've already got in the doc_strings into the html docs.

Copy link
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

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

Thanks for this @dschult , LGTM! I think this captures the most important components of the backend docs reorg - exposing functionality and pulling it into it's own refguide "article". Additional updates to the content of the docs themselves can be handled in follow-up PRs!

@Schefflera-Arboricola
Copy link
Contributor

The docs for the constant config are appearing just as plain text without much context(we should probably put 1-2 lines explaining this):

alias of NetworkXConfig(backend_priority=[], backends=Config(cugraph=Config(), graphblas=Config(), parallel=Config()), cache_converted_graphs=False)

Also, I think adding the docstring of the base Config class and the docstring of the _dispatchable class to the user-facing docs might also be helpful.

@dschult
Copy link
Member Author

dschult commented Apr 4, 2024

I put the base class Config into this PR now... Thanks for pointing that out. And I agree with your other comments @Schefflera-Arboricola, but I wasn't able to get numpydoc to handle those cases nicely. I am hoping to be able to get this into release 3.3. And I don't like putting docs directly into the rst files (they are then separated from the code). It would be nice though to get the docs in the code to appear in the html docs when they exist though. :) So, I've added Config. But config doesn't have any docs in the code. It is just an instance of NetworkXConfig. So this text seems to me good for now.

More details:
The attribute config text comes from the autoclass feature of numpydocs. I liked it better than the handlers e.g. autodata or autofunction which showed exactly the same things that NetworkXConfig displays. At least this correctly shows it is a NetworkXConfig. And you can tell it is the same without having to compare the entire doc_strings for the two objects. I've decided that it is better to have the text saying it is an alias than docs which made it look like this was a class itself.

The _dispatchable class docstring doesn't appear in the docs with autoclass or autofunction or autodecorator. Only the docstring for _dispatchable.__new__. While it is clearly defined as a class, numpydoc is not able to handle it using autoclass. It raises an error:

Handler <function mangle_docstrings at 0x104b385e0> for event 'autodoc-process-docstring' threw an exception (exception: Expected a class or None, but got <function _dispatchable at 0x103680860>)

What do you think of my inclusion of Config, and changing the _decorator doc to use autodecorator (the only change I can see is that the function name has a @ in front of it. :)

@dschult
Copy link
Member Author

dschult commented Apr 5, 2024

Based on the review comments above and discussions at the meetings this week, I have made a few more changes:

  • I added most of the contents of the _dispatchable class (which don't show up in html) to the __new__ method's doc_string so it shows up.
  • I added description of how to use environment variables to run a backend, and I added language to describe what happens with environment variables, with backend= keyword args and with input that is a converted backend graph.
  • This led me to wordsmith a little bit in the docs of the backends module.
  • I moved the tests for backends into the utilities folder. (They were in classes for historical reasons).

@dschult
Copy link
Member Author

dschult commented Apr 5, 2024

Looks like the tests are failing due to chocolatey's webpage not responding. :)

@dschult dschult merged commit 83d2cf4 into networkx:main Apr 5, 2024
42 checks passed
@dschult dschult deleted the move_backend_docs branch April 5, 2024 04:58
@jarrodmillman jarrodmillman added this to the 3.3 milestone Apr 5, 2024
cvanelteren pushed a commit to cvanelteren/networkx that referenced this pull request Apr 22, 2024
…idebar entry. (networkx#7389)

* make doc sidebar entry for backends and config

* put _dispatchable into section heading name for ease of doc search

* add Config class to the docs

* Change docs based on review comments and meeting discussions

* move backend tests from classes/tests directory to utilities/test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants