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

Convert demo READMEs to Sphinx index pages #1391

Merged
merged 5 commits into from Apr 30, 2020
Merged

Conversation

huonw
Copy link
Member

@huonw huonw commented Apr 28, 2020

This moves us towards using Read the Docs as the main source of demos, by duplicating the indexing information from READMEs into the Sphinx docs.

We've decided to move to using Read the Docs like this because:

I used m2r to convert each of our demo READMEs to a reStructuredText index.txt file, and then fixed them up by hand. For each index, I've gone with:

Title
======

... description with subsections etc ...

Table of contents
------------------

... full ToC listing of subfolders/demos of this index ...

(Where the description is empty in some cases, for directories without READMEs: #1139.)

The thinking is we'll replace the README.md files with a much simplified set of information, potentially just a link to the corresponding Read the Docs. However, the READMEs are still useful as an index if someone has a copy of the demos locally in Jupyter lab. Thus, this PR also extends the demo_table.py script to render the demo indexing table to both a raw-HTML markdown version into demos/README.md and a rST list-table version into docs/demos/index.txt. This means we can still retain that demo table in an otherwise reduced/not-human-maintained demos/README.md.

Other than extending demo_table.py, this tries to do the minimal work to get this working. Future work:

Rendered demo landing page: https://stellargraph--1391.org.readthedocs.build/en/1391/demos/index.html

See: #1298

@codeclimate
Copy link

codeclimate bot commented Apr 28, 2020

Code Climate has analyzed commit 41165d4 and detected 5 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1
Security 4

View more on Code Climate.

for algorithm in algorithms:
with builder.element("tr"):
for heading in headings:
with builder.element("td"):
Copy link

Choose a reason for hiding this comment

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

Avoid deeply nested control flow statements.

@@ -81,6 +87,8 @@ def __init__(self, text=None, link=None, details=None):
self.text = text
self.link = link
self.details = details
self.kind = kind
assert isinstance(kind, LinkKind)
Copy link

Choose a reason for hiding this comment

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

Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.

@huonw huonw marked this pull request as ready for review April 29, 2020 02:18
huonw added a commit that referenced this pull request Apr 30, 2020
…1398)

The raw HTML table was not converting to reStructuredText through `nbsphinx`
(which, I think, uses `jupyter nbconvert --to=rst`, which uses pandoc). The
output lost the links + images (see more details about `pandoc` in #1398).

This PR updates the links to be normal markdown links + images, which ensures
they do get converted correctly. To distinguish these links from normal body
text, these are formatted as a block-quote (`> ...`). This gives acceptable (not
great) formatting on each of the major platforms one might be using to look at
this (see images in #1398):

- Jupyter locally
- Github:
  https://github.com/stellargraph/stellargraph/blob/2972637ec3b75c7ab7bc6a7a96697489b03f62fd/demos/basics/loading-networkx.ipynb
- nbviewer:
  https://nbviewer.jupyter.org/github/stellargraph/stellargraph/blob/2972637ec3b75c7ab7bc6a7a96697489b03f62fd/demos/basics/loading-networkx.ipynb
- Read the Docs:
  https://stellargraph--1398.org.readthedocs.build/en/1398/demos/basics/loading-networkx.html


Note that both Github and nbviewer have CSS that forces images to be formatted
as block elements, and so they're positioned separately (instead of inline
elements as is the default). This is unfortunate, but per #1391, we're moving
towards Read the Docs being the main online display of our demos.

We can potentially improve the Read the Docs formatting with custom CSS or some
post-processing (similar to #1401).

See: #1293
Copy link
Contributor

@timpitman timpitman left a comment

Choose a reason for hiding this comment

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

This is looking good - the index files for the different demo folders looked correct. On the overall demo index (https://stellargraph--1391.org.readthedocs.build/en/1391/demos/index.html) , the table doesn't have hover hints, so is a bit hard to read - do we need longer names for the headings again?

Copy link
Contributor

@kieranricardo kieranricardo left a comment

Choose a reason for hiding this comment

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

The index.txt files look good to me 👍

huonw added a commit that referenced this pull request Apr 30, 2020
Our demo notebooks contain links to READMEs. These links don't work on Read the Docs/in Sphinx, because we have `index` (per the web server convention) files for each directory, not READMEs.

This PR writes a basic sphinx extension that rewrites any relative links to a file called "README.md" to instead be a link to "index.txt", matching our Sphinx index files. I modelled this after:

- https://www.sphinx-doc.org/en/master/extdev/index.html#dev-extensions
- https://github.com/spatialaudio/nbsphinx/blob/93398b9d8ba3922e860486d7eacf9eeb3b036580/src/nbsphinx.py#L1527-L1586

This approach allows us to retain links to READMEs in the notebooks, so that they work locally and on Github. These links are probably useful even with the move to Read the Docs being the canonical display of demos (#1391), due to being useful locally. (A simpler approach that breaks this would be to change the links to be correct for Read the Docs within each notebook.)

This also adjusts the case of one of the `connector/neo4j` README file, so that it's consistent with the rest of the repo, to make this code simpler.

This fixes 12 out of 66 of the remaining warnings in #1360; all of the ones like:

```
WARNING: File not found: 'demos/README.md'
WARNING: File not found: 'demos/node-classification/README.md'
```

Example: in the GCN node classification notebook (https://github.com/stellargraph/stellargraph/blob/1cca6b6c6f4cd1fea7c6932918cba18d15eef6d4/demos/node-classification/gcn/gcn-cora-node-classification-example.ipynb), there's a conclusion that includes links to both the parent node-classification README and the all notebooks README. Its Markdown source is:

```markdown
StellarGraph includes [other algorithms for node classification](../README.md) and [algorithms and demos for other tasks](../../README.md).
```

Before: https://stellargraph.readthedocs.io/en/latest/demos/node-classification/gcn/gcn-cora-node-classification-example.html#Conclusion These links stayed pointing to `README.md` in the respective parent directories, which don't exist in the Sphinx docs/on Read the Docs, and so they're dead links. Generated HTML (note the `external` CSS class, and the `href=.../README.md`):

```html
StellarGraph includes <a class="reference external" href="../README.md">other algorithms for node classification</a> and <a class="reference external" href="../../README.md">algorithms and demos for other tasks</a>.
```

After: https://stellargraph--1401.org.readthedocs.build/en/1401/demos/node-classification/gcn/gcn-cora-node-classification-example.html#Conclusion The links were rewritten and now work. Generated HTML (note the `internal` CSS class, and the `href=.../index.html`):

```html
StellarGraph includes <a class="reference internal" href="../index.html"><span class="doc">other algorithms for node classification</span></a> and <a class="reference internal" href="../../index.html"><span class="doc">algorithms and demos for other tasks</span></a>.
```

See: #1292
@huonw
Copy link
Member Author

huonw commented Apr 30, 2020

do we need longer names for the headings again?

Yeah, I'm going to open a follow up issue covering this and other bits from the PR description:

  • optimising the display of each of the tables (at the moment they scroll sideways a lot)
  • handling abbreviations in the main indexing table, because it doesn't seem easy to get hover text in rST
  • updating the README files to remove their content
  • updating any links to the demos on GitHub to point to Read the Docs

(The other future work there should be handled by other issues.)

@huonw
Copy link
Member Author

huonw commented Apr 30, 2020

I opened #1417, #1418, #1419, #1420.

@huonw huonw merged commit 7610278 into develop Apr 30, 2020
@huonw huonw deleted the feature/1298-demo-readmes branch April 30, 2020 22:53
huonw added a commit that referenced this pull request May 4, 2020
This augments the `demo_table.py` script (and thus renames it to
`demo_indexing.py`) to support adding:

- a link to the corresponding Read the Docs page for the task
  (e.g. `demos/node-classification/README.md` points to
  https://stellargraph.readthedocs.io/en/latest/demos/node-classification/index.html
  )
- a listing of each demo within the subfolder, including both a relative link
  ("open here") and a link to the Read the Docs version ("rendered")

This is designed to provide good guidance for a user to get to our nicely
displayed demos on Read the Docs, and also provide them with basic indexing when
opening the READMEs while actually running a demo (either in a text editor etc.,
or in Jupyter Lab).

The other contents is removed except for the title, since that content has been
moved to Read the Docs (#1391).

Finally, the `demos/link-prediction/random-walks/utils/node2vec/README.md`
README was removed. It is inherited directly from
https://github.com/aditya-grover/node2vec and doesn't convey relevant content,
and there's a separate LICENSE file that fulfils our legal obligation with that
included code.

Example: https://github.com/stellargraph/stellargraph/tree/f18209f5d89b1874e68906a23961a170d899d733/demos/node-classification

Top level entry point: https://github.com/stellargraph/stellargraph/tree/f18209f5d89b1874e68906a23961a170d899d733/demos

(See #1433 for screenshot of Jupyter Lab)

See: #1419
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.

None yet

3 participants