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

DOC: Minor edits/suggestions from a full runthrough of documentation #121

Closed
wants to merge 10 commits into from

Conversation

rossbar
Copy link
Contributor

@rossbar rossbar commented Mar 19, 2020

In preparation for another round of converting Elegant Scipy chapters to MyST-flavored markdown, I did a full read-through of the documentation to get caught up on the latest changes/features.

While working my way through the docs (which are really great BTW), I made a couple minor changes mostly of a grammatical nature.

One set of non-grammatical changes that deserve special attention are those in ff7d040. According to the docs, it should be allowed to have a space between bracketed reference links in markdown syntax, i.e. [name] [key] is equally as valid as [name][key]; however, this does not appear to work as expected (or at least not according to my interpretation of the docs). I've never used that syntax with markdown before, so I don't know if this is a MyST issue or a documentation issue - I just thought I'd draw extra attention to it.

In addition to the changes, I had a few comments additional comments:

  • The intersphinx links in using/syntax.md don't appear to have worked on the deployed docs. They work when building locally though, so maybe the version deployed to readthedocs needs to be updated.
  • The using/benchmark.md could maybe be titled differently or split into two separate pages. There is a brief section at the top of the page that discusses benchmarking/performance, followed by a much longer chunk of unrelated-ish text presenting an overview of markdown. From an organizational standpoint it might be nice to have an "Overview of markdown" page and a separate "Performance" page.
  • This may need to be a separate issue, but it seems like the blockquotes are not currently working. I see the same behavior when building locally (with 21d7e1e). I'm happy to open an issue to provide more detail.

@chrisjsewell
Copy link
Member

Thank @rossbar I'll work through this. Also be sure to check out the new VS Code extension and let us know any feedback 😁

@chrisjsewell
Copy link
Member

it should be allowed to have a space between bracketed reference links in markdown syntax, i.e. [name] [key] is equally as valid as [name][key]

Yeh this is not true (referring to https://spec.commonmark.org/dingus/) and should be changed in the docs. I think @choldgraf added it, so perhaps he could clarify the intended meaning

The intersphinx links in using/syntax.md don't appear to have worked

I can't replicate this; can you specify a certain link to look at.

The using/benchmark.md could maybe be titled differently or split into two separate pages... followed by a much longer chunk of unrelated-ish text presenting an overview of markdown.

Yeh fair. the large chunk is there, becuse that is what the performance tests are actually run on (so it has to be 'large'). But I think this can be separated into a separate document and just hid in the documentation (with orphan=True), and linked to in the performance section.

This may need to be a separate issue, but it seems like the blockquotes are not currently working.

Yeh a seperate issue would probably be best, with a minimal example of what is not working, with a screenshot, of how it currently renders, and explain how you would expect it to render.

@chrisjsewell
Copy link
Member

Note the circle CI issue is related to pydata/pydata-sphinx-theme#102 (comment)

@chrisjsewell
Copy link
Member

chrisjsewell commented Mar 19, 2020

Ah actually I see the intersphinx issue, is related to referencing the mistletoe docs (which I wrote): https://readthedocs.org/projects/myst-parser/builds/10642528/. Its strange because this build was yesterday, but the last mistletoe build was a week ago: https://readthedocs.org/projects/mistletoe-ebp/builds/ and contains these 'targets'

@choldgraf
Copy link
Member

choldgraf commented Mar 19, 2020

re: the extra space ([] []), I don't remember writing it, I'm happy to go w/ whatever is correct

@chrisjsewell
Copy link
Member

@choldgraf can you try rerunning the readthedocs ta

@choldgraf
Copy link
Member

done!

@chrisjsewell
Copy link
Member

Yep that fixed it 👍

@rossbar
Copy link
Contributor Author

rossbar commented Mar 20, 2020

The intersphinx links in using/syntax.md don't appear to have worked

I can't replicate this; can you specify a certain link to look at.

Sorry, I should've been more specific - glad the RTD rerun fixed it.

Note the circle CI issue is related to pydata/pydata-sphinx-theme#102 (comment)

If I understood the linked comment (and subsequent discussion) correctly, this should be taken care of in 268dd9e - please make sure that I organized it the way that you prefer though!

@rossbar
Copy link
Contributor Author

rossbar commented Mar 20, 2020

This may need to be a separate issue, but it seems like the blockquotes are not currently working.

Yeh a seperate issue would probably be best, with a minimal example of what is not working, with a screenshot, of how it currently renders, and explain how you would expect it to render.

It turns out this is theme/style issue - the pydata-sphinx-theme doesn't indent or change the formatting really at all for block quotes (yet). For example, the block quotes examples in the docs don't actually look different than paragraphs, and because there is no indenting, the nested blockquote example looks "broken".

@chrisjsewell
Copy link
Member

chrisjsewell commented Mar 20, 2020

If I understood the linked comment (and subsequent discussion) correctly, this should be taken care of in 268dd9e - please make sure that I organized it the way that you prefer though!

You just need to also remove requirements.txt from .readthdocs.yml

@chrisjsewell
Copy link
Member

the pydata-sphinx-theme doesn't indent or change the formatting really at all for block quotes (yet)

yes, I believe that is awaiting pydata/pydata-sphinx-theme#103

Updated conf.py and setup.py rtd extras to depend on
pydata-sphinx-theme (recent name change).

Removed requirements.txt from docs, which only contained
a pointer to the repo for pandas_sphinx_theme

Updated circleCI and readthedocs configurations to remove
doc/requirements.txt
@rossbar
Copy link
Contributor Author

rossbar commented Mar 20, 2020

You just need to also remove requirements.txt from .readthdocs.yml

oops - done in a7de620

@jstac
Copy link
Member

jstac commented Mar 20, 2020

Fixes #122

@chrisjsewell chrisjsewell linked an issue Mar 20, 2020 that may be closed by this pull request
chrisjsewell added a commit that referenced this pull request Mar 28, 2020
chrisjsewell added a commit that referenced this pull request Apr 1, 2020
This commit implements the move from `mistletoe` to `markdown-it-py`, as the underlying markdown parser. The reason for this is are discussed in executablebooks/meta#44 (comment) and executablebooks/meta#47, and the PR #123 discusses in more details the update.

Additional changes:

- Update `pydata-sphinx-theme` requirement
- Improve testing and move to GitHub Actions CI
- Add tests and fixes for reporter warnings and include directive
- Add documentation of sphinx parser options
- Apply doc fixes suggested by @rossbar in #121
- Add warning for non-consecutive headings
@chrisjsewell
Copy link
Member

These changes I believe have been addressed in #123 and in particular 95a0404. But feel free to reopen/re-PR if not 😄

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.

Install instructions failing
4 participants