Updating requirements for myst-parser #1197

choldgraf · 2021-02-04T03:37:01Z

This is a first-pass PR to get some low-hanging fruit out of the way. It does the following things:

Updates the requirements file to use the myst-parser instead of recommonmark
Updates all of the eval_rst blocks to instead use {eval-rst} directives, which are the MyST-equivalent of the same thing
- This should mean that these docs behave the same was as they did with recommonmark.
- However, there are likely going to be a few places where the behavior isn't quite the same (e.g., I think recommonmark lets you link directly to headers via [sometext](page#header)). Will need to find these and update them.
Updates the index page to use MyST markdown syntax, just to give you an idea of what this change will look like elsewhere as well.

supercedes #1192

choldgraf · 2021-02-04T03:41:22Z

I think that the cherry-pick worked OK! Will start trying to figure out errors.

BTW, @jpmckinney - I'm trying to locally run the command that the CI build is failing on, but it is giving me a different command locally. I wonder if there's an easy "gotcha" that I am not noticing:

$ sphinx-build -q -b dirhtml docs build/es -D language="es"

Exception occurred:
  File "/home/choldgraf/anaconda/envs/ocds/lib/python3.6/gettext.py", line 514, in translation
    raise OSError(ENOENT, 'No translation file found for domain', domain)
FileNotFoundError: [Errno 2] No translation file found for domain: 'schema'

I will dig into the docs a bit closer next time to see if there's more in there about building the translations. I don't have a ton of familiarity with translation workflows in Sphinx

jpmckinney · 2021-02-04T05:23:22Z

There are dependencies between commands (eg I think this error is because the schema message catalog hasn’t been compiled; it’s used by PyBabel to translate JSON files - see the translate method call in conf.py) but if you run make, it will run all commands as needed.

There are many Make targets - if you just want to build one language for example.

choldgraf · 2021-02-04T05:37:58Z

Gotcha, thanks. This is probably the most formidable Makefile I've ever used 😅 I am very impressed and a little fearful!

chrisjsewell · 2021-02-05T23:41:44Z

I would suggest turning on fail on errors when building the documentation at e.g.:

standard/include/common.mk

Line 99 in 323dee4

sphinx-build -q -b dirhtml $(DOCS_DIR) $(BUILD_DIR)/en

sphinx-build -nW --keep-going -q -b dirhtml $(DOCS_DIR) $(BUILD_DIR)/en

currently there are a bunch of warnings in the documentation build (even before this PR): https://github.com/open-contracting/standard/runs/1810749509

jpmckinney · 2021-02-06T01:14:54Z

I'm making some updates to one of our Sphinx extensions to not warn about things that are actually fine.

However, some of the warnings I don't know how to quiet. For example:

/home/runner/work/standard/standard/docs/schema/codelists.md:18:<translated>:1: WARNING: None:any reference target not found: ../../release-schema.json

When we build the documentation, we write some JSON and CSV files to the build directory that Sphinx doesn't know about, so it complains about "target not found".

I figure there's some way to tell Sphinx about those files. I've seen env.note_dependency around - not sure if that's the trick I'm missing.

choldgraf · 2021-02-06T01:21:41Z

@jpmckinney would it be acceptable to use the {download} role for this? I think that this would suppress these warnings. E.g.:

Here is my {download}`schema <../../release-schema.json>`!

This is used to link directly to assets that are not "part of the documentation" but are meant to be downloaded

jpmckinney · 2021-02-06T01:40:17Z

Aha! That sounds like the solution. I didn't know about that role (for my later reading: https://www.sphinx-doc.org/en/master/usage/restructuredtext/roles.html#referencing-downloadable-files)

…ndard into myst-parser-switch

jpmckinney · 2021-02-12T04:27:43Z

@choldgraf Sphinx merged my PR (sphinx-doc/sphinx#8853) for heading level issue (executablebooks/MyST-Parser#302), so we can pin to a commit in Sphinx until the next 3.x release is made.

choldgraf · 2021-02-12T04:28:42Z

Wow, nice! Will add that to the next commit. Right now I am trying to figure out this weird bug that we seem to be triggering in the markdown-it-py library when doing translations.

choldgraf · 2021-02-12T04:29:11Z

looking at your PR there, I wonder if that actually fixes our bug? Will investigate

jpmckinney · 2021-02-12T04:30:22Z

looking at your PR there, I wonder if that actually fixes our bug? Will investigate

The PR makes the heading level go from 0 to 1 instead of 0 to 2, so it should fix the immediate problem.

jpmckinney · 2021-02-12T04:32:16Z

Wow, nice! Will add that to the next commit. Right now I am trying to figure out this weird bug that we seem to be triggering in the markdown-it-py library when doing translations.

I think the following error is fixed if you upgrade mdit-py-plugins to 0.2.5 (released for this fix executablebooks/mdit-py-plugins#11)

  File "/opt/hostedtoolcache/Python/3.6.12/x64/lib/python3.6/site-packages/mdit_py_plugins/front_matter/index.py", line 51, in frontMatter
    if marker_str[(pos - start) % marker_len] != state.src[pos]:
IndexError: string index out of range

choldgraf · 2021-02-12T04:43:27Z

@jpmckinney ahh I just spent way too long trying to figure out what the heck was going on with that bug haha, I didn't realize that was the package to upgrade, I knew it had been addressed somewhere.

FWIW this commit is what I finally used as a workaround, and I have no idea why it works 😅 dcd9b4b (#1197)

OK I reverted that change and pushed another commit that updates mdit-py-plugins.

choldgraf · 2021-02-12T04:50:59Z

@jpmckinney any thoughts on this one? Latest error seems to be some custom docs code in this repo?

https://github.com/open-contracting/standard/pull/1197/checks?check_run_id=1884827925#step:6:64

jpmckinney · 2021-02-12T05:04:08Z

@jpmckinney any thoughts on this one? Latest error seems to be some custom docs code in this repo?

https://github.com/open-contracting/standard/pull/1197/checks?check_run_id=1884827925#step:6:64

Oh, yeah, I'll have to edit that script for the new theme (should be easy - just changing a selector). For now, you can comment out python util/add_translation_notes.py in ci.yml. What it does is, if any translations are missing, it adds an admonition (as HTML) to the page to say, "hey, this page has been updated in English, but we're showing you the most recent Spanish version for now" more or less.

It's possible some of the tests in the tests/ directory will start failing next, if their selectors don't match the theme.

I'll also need to customize the theme later, because we replace the search page's JavaScript to send a query to Elasticsearch. (The deployment process creates an Elasticsearch index.)

choldgraf · 2021-02-12T05:05:38Z

@jpmckinney sounds good - I don't have any more cycles for this one tonight, so I'll just comment that line out and see what breaks next but then gonna log off :-)

Are you using ElasticSearch just to speed things up? Nifty...

jpmckinney · 2021-02-12T05:07:15Z

Are you using ElasticSearch just to speed things up? Nifty...

The quality of Sphinx's search results are not great, so we use Elasticsearch, which supports quoted phrases, etc. and we're able to tune the results better.

choldgraf · 2021-02-12T05:09:12Z

@jpmckinney you ever write up your workflow there in a blog post or something? That is pretty cool, a lot of folks ask for improved search in Jupyter Book and I've been trying to figure out ways that we could recommend something, or improve the tech itself

Curious if you ever played around with using something like lunr

jpmckinney · 2021-02-12T05:14:16Z

Interesting - I'll have a look at Lunr.

I haven't written it up, but https://ocds-index.readthedocs.io/en/latest/ is the Python package that does the work (at the Python-level, it's possible to configure it for different themes).

ocds-index's CLI gets called by this script, which is run by GitHub Actions once the tests pass: https://github.com/open-contracting/deploy/blob/master/deploy-docs.sh#L8-L9

Then, the theme queries Elasticsearch: https://github.com/open-contracting/standard_theme/blob/open_contracting/standard_theme/static/js/search.js#L20-L53

We use https://readonlyrest.com to expose Elasticsearch to the web safely. (You'll notice the JavaScript in the theme uses Basic Authentication nonetheless, just so that random bots don't hit Elasticsearch.)

choldgraf · 2021-02-12T05:29:06Z

Very cool, thanks for sharing!

Note: I'm not sure what the latest failure is. Though tests seem to pass 🙂

https://github.com/open-contracting/standard/pull/1197/checks?check_run_id=1884900587#step:8:1

jpmckinney · 2021-02-12T05:35:42Z

Oh, it's because this branch is from your fork, rather than in the repo itself, and an ssh-related GitHub Action isn't letting the build access the repo's secrets for that reason.

You said you'd be logging off for the evening 😛 but if you change your origin and push a new branch to the repo directly, then it should build!

choldgraf · 2021-02-12T14:38:54Z

@jpmckinney - hmmm - I don't believe that I have permission to push directly to the standard repo:

$ git push upstream myst-parser-switch
remote: Permission to open-contracting/standard.git denied to choldgraf.
fatal: unable to access 'https://github.com/open-contracting/standard/': The requested URL returned error: 403

jpmckinney · 2021-02-12T14:46:17Z

Oops, I thought I had already invited you. I've sent an invite now.

choldgraf added 5 commits February 3, 2021 11:36

Updating requirements for myst-parser

733f6a8

Using MyST markdown eval-rst directive

d5c466c

Updating header metadata to MyST syntax

fc1cdcb

Converting index page to MyST

3e19828

Fixing markdown header levels

b4ad655

conf: Disable dollarmath

c1ab0a7

jpmckinney mentioned this pull request Feb 5, 2021

Issues with sphinx Locale transform (assumes rST) executablebooks/MyST-Parser#302

Open

choldgraf added 2 commits February 11, 2021 20:00

Fixing references to downloadable JSON files.

9b542ba

Merge branch 'myst-parser-switch' of https://github.com/choldgraf/sta…

bd0e2e6

…ndard into myst-parser-switch

Pinning Sphinx to commit to avoid header level error.

667b4e8

Updating MDIt-py plugins for bugfix.

2c6150d

choldgraf force-pushed the myst-parser-switch branch from dcd9b4b to 2c6150d Compare February 12, 2021 04:46

Properly linking another JSON file.

f72e811

Disabling a CI test that fails because of MyST.

e76a6ba

jpmckinney mentioned this pull request Feb 12, 2021

Updating requirements for myst-parser #1213

Merged

jpmckinney closed this Feb 12, 2021

jpmckinney added this to the Iterative improvements milestone Mar 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updating requirements for myst-parser #1197

Updating requirements for myst-parser #1197

choldgraf commented Feb 4, 2021 •

edited

Loading

choldgraf commented Feb 4, 2021

jpmckinney commented Feb 4, 2021

choldgraf commented Feb 4, 2021

chrisjsewell commented Feb 5, 2021 •

edited

Loading

jpmckinney commented Feb 6, 2021

choldgraf commented Feb 6, 2021 •

edited

Loading

jpmckinney commented Feb 6, 2021

jpmckinney commented Feb 12, 2021

choldgraf commented Feb 12, 2021

choldgraf commented Feb 12, 2021 •

edited

Loading

jpmckinney commented Feb 12, 2021

jpmckinney commented Feb 12, 2021 •

edited

Loading

choldgraf commented Feb 12, 2021 •

edited

Loading

choldgraf commented Feb 12, 2021

jpmckinney commented Feb 12, 2021 •

edited

Loading

choldgraf commented Feb 12, 2021

jpmckinney commented Feb 12, 2021

choldgraf commented Feb 12, 2021

jpmckinney commented Feb 12, 2021

choldgraf commented Feb 12, 2021

jpmckinney commented Feb 12, 2021

choldgraf commented Feb 12, 2021

jpmckinney commented Feb 12, 2021

Updating requirements for myst-parser #1197

Updating requirements for myst-parser #1197

Conversation

choldgraf commented Feb 4, 2021 • edited Loading

choldgraf commented Feb 4, 2021

jpmckinney commented Feb 4, 2021

choldgraf commented Feb 4, 2021

chrisjsewell commented Feb 5, 2021 • edited Loading

jpmckinney commented Feb 6, 2021

choldgraf commented Feb 6, 2021 • edited Loading

jpmckinney commented Feb 6, 2021

jpmckinney commented Feb 12, 2021

choldgraf commented Feb 12, 2021

choldgraf commented Feb 12, 2021 • edited Loading

jpmckinney commented Feb 12, 2021

jpmckinney commented Feb 12, 2021 • edited Loading

choldgraf commented Feb 12, 2021 • edited Loading

choldgraf commented Feb 12, 2021

jpmckinney commented Feb 12, 2021 • edited Loading

choldgraf commented Feb 12, 2021

jpmckinney commented Feb 12, 2021

choldgraf commented Feb 12, 2021

jpmckinney commented Feb 12, 2021

choldgraf commented Feb 12, 2021

jpmckinney commented Feb 12, 2021

choldgraf commented Feb 12, 2021

jpmckinney commented Feb 12, 2021

choldgraf commented Feb 4, 2021 •

edited

Loading

chrisjsewell commented Feb 5, 2021 •

edited

Loading

choldgraf commented Feb 6, 2021 •

edited

Loading

choldgraf commented Feb 12, 2021 •

edited

Loading

jpmckinney commented Feb 12, 2021 •

edited

Loading

choldgraf commented Feb 12, 2021 •

edited

Loading

jpmckinney commented Feb 12, 2021 •

edited

Loading