[MNT,DOC] use latest code in builds of the docs with RTD #472

stevejpurves · 2021-09-07T14:35:16Z

This PR aims to address #285 and change the default docs build to always use the version of thebe represented by the latest code in the repo. Previously the docs would load the latest published version from unpkg.

TODOS:

add makefile commands to build the docs locally, and place a copy in docs/_static
update docs to use the local build
update readthedocs builds as needed and ensure that these build with the latest local thebe

humitos · 2021-09-16T15:36:10Z

hi @stevejpurves! I'm testing this PR but I'm getting this error:

repos/thebe/docs  pr/472 ✗                                                                                                             2m ⚑  
▶ make html
rm -rf _static/lib
cd ..; yarn install --frozen-lockfile; yarn build
yarn install v1.22.11
[1/5] Validating package.json...
[2/5] Resolving packages...
error Your lockfile needs to be updated, but yarn was run with `--frozen-lockfile`.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
yarn run v1.22.11
$ webpack --mode development --progress
/bin/sh: line 1: webpack: command not found
error Command failed with exit code 127.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
make: *** [Makefile:43: js] Error 127
Time: 0h:00m:03s

it seems that yarn is not installing the dependencies and failing. I don't have too much experience with yarn and I'm not sure if it's possible to remove --frozen-lockfile argument.

I'm facing the same problem when testing this inside Read the Docs.

humitos · 2021-09-16T16:48:29Z

For the record, I was able to make this PR work with the new Read the Docs Docker images and config file (see readthedocs/readthedocs.org#8453, readthedocs/readthedocs.org#8478, and readthedocs/readthedocs-docker-images#107) with the following changes.

Read the Docs configuration file

# readthedocs.yml
version: 2

python:
  install:
    - requirements: docs/doc-requirements.txt

build:
  os: "ubuntu20"
  languages:
    python: "3.9"
    nodejs: "14"

Note the usage of build.os and build.languages (these name may change, we are still discussing them); but this is going to be the way to install nodejs, for example.

Sphinx configuration file

# docs/conf.py
path_root = Path(__file__).parent.parent
os.environ['PATH'] = f'{path_root}/node_modules/.bin/:' + os.environ["PATH"]

run(["npm", "install", "yarn", "jsdoc"], cwd=path_root)
run(["yarn", "--version"], cwd=path_root)
run(["yarn", "install"], cwd=path_root)
run(["yarn", "build",], cwd=path_root)
sh.copytree(f"{path_root}/lib", "_static/lib")

Highlights:

I'm adding local node_modules/.bin to the PATH environment variable
I removed the --frozen-lockfile
I'm installing jsdoc that's required (not available by default in new Docker images)
There is no need to call node_modules/yarn/bin/yarn, just yarn works

I'll let you know when we deploy these changes to production in beta so you can start testing this and giving us some feedback 😍

stevejpurves · 2021-09-16T19:45:06Z

@humitos that's great, thank you!

do you have a rough ETA on those changes making it to prod/beta?

humitos · 2021-09-20T11:05:53Z

do you have a rough ETA on those changes making it to prod/beta?

Not yet. We are currently working to make this happen.

humitos · 2021-09-29T07:31:32Z

Hi! We already deployed these changes. You should be able to use build.os and build.tools as I mentioned in #472 (comment). Please, let me know if it works for you! 💪🏼

choldgraf · 2021-10-02T22:13:24Z

That's great news @humitos !!

@stevejpurves just lemme know if you'd like me to take a look or provide feedback on something!

stevejpurves · 2021-10-04T12:02:43Z

ok, RTD build is green now. Guess I am surprised by yarn install seemingly cleaning out the node_modules directory and making yarn, jsdoc subsequently unavailable. With or without the frozen-lockfile option. I can't reproduce this locally, but that's the apparent behaviour on RTD.

Going to do a little more to try and make local dev cycle, nicer while keeping RTD up

…version bumps

choldgraf

This in general looks pretty nice to me! Just a few comments about the build process and the docs, happy to take another look or iterate on comments! Thanks @stevejpurves for doing this work :-)

choldgraf · 2021-10-06T19:35:37Z

docs/conf.py

+# during development and testing
+path_root = Path(__file__).parent.parent
+
+if os.environ.get('READ_THE_DOCS'):


What's the reason for having two different workflows on RTD vs. locally? I'm wondering if it'd be possible to just have one check for whether the _static/lib folder exists rather than making it build environment-specific 🤔

That's a good point @choldgraf.

I'm seeing the RTD process as separate mostly due to the unique setup it needs i.e. because of the local installs of yarn and jsdoc which are typically going to be global installs in someone's local development environment. We don't want two versions of these kicking around locally, or to add them to the developer's path.

I have separated that environment setup from the build commands, where the latter are now a run based on a check for the _static/lib in both environments as you suggested.

Note in the course of this I've picked up on some more changes that were needed to properly support the "use local build" both locally and on RTD. see changes to Makefile and webpack.config.js.

docs/conf.py

choldgraf · 2021-10-06T19:38:22Z

docs/conf.py

+if os.environ.get('READ_THE_DOCS'):
+    node_modules_bin = f'{path_root}/node_modules/.bin/';
+    os.environ['PATH'] = f'{node_modules_bin}:' + os.environ["PATH"]
+    run(["npm", "install", "yarn", "jsdoc"], cwd=path_root)


I wonder if we could use tox or nox to handle the environment installs for these steps 🤔 but I don't think it needs to block this PR

I'm open to it @choldgraf but don't understand how it fits yet. With this library being primarily driven by a js/yarn build process, would it help? as this wouldn't be included in any virtualenv dependency management (or does that go beyond python dependencies?). I've not used tox or nox though so happy to chat about this to get a better understanding.

just as a point of info - nox supports conda environments so you can use it with JS-style build systems, which is pretty nice. But I don't think it needs to block this! For example: https://github.com/pydata/pydata-sphinx-theme/blob/master/noxfile.py

choldgraf · 2021-10-06T19:39:35Z

docs/examples/matplotlib_interact_example.rst

@@ -67,7 +67,7 @@ Example
       },
     }
   </script>
-   <script src="https://unpkg.com/thebe@latest/lib/index.js"></script>
+   <script src="../_static/lib/index.js"></script>


hmmm - is it the case that these example HTML files will never work unless they are in the _static folder (since the library won't be in the right path otherwise)? If so, then why don't we just put the whole folder in _static/html_examples and add lots of links to that folder in docs + README so that people discover it?

It would simplify that path, as we would be loading from ../lib/index.js as per the other pure HTML examples that we are copying.

In this case this example needs sphinx to compile it to HTML first -- I'm not clear on how or when to move the result of that build into the HTML though?

choldgraf · 2021-10-06T19:42:35Z

examples/demo-active.html

@@ -14,10 +14,12 @@
        },
      }
    </script>
+    <!-- uncomment to use latest thebe release


could we just add this as a note in the docs + README, rather than putting it in a single example? e.g.

:::{admonition} Use the latest release of `thebe` on unpkg :class: tip These examples build a _local_ version of `thebe` in order to show off the latest features. If you'd like to instead load the latest _release_ of Thebe, replace the `<script>` elements with the following: ```html <script type="text/javascript" src="https://unpkg.com/thebe@latest"></script> ``` :::

choldgraf · 2021-10-12T03:33:35Z

@stevejpurves is this one blocked on anything? Just wanna make sure that you're not waiting for others for something

stevejpurves · 2021-10-12T14:08:00Z

docs/Makefile

+js-dev: clean-js
+	cd ..; yarn install; yarn build
+	cp -R ../lib _static/lib
+


New js build rules provided but not invoked by default

stevejpurves · 2021-10-12T14:08:30Z

package.json

@@ -7,10 +7,11 @@
    "build": "webpack --mode development --progress",
    "build:prod": "webpack --mode production",
    "build:watch": "webpack --mode development --progress --color --watch",
-    "prepare": "yarn run build:prod",
+    "prePublishOnly": "NODE_PREPUBLISH=true yarn run build:prod",


Added environment var to identify npm publish action in webpack.config.js

stevejpurves · 2021-10-12T14:11:04Z

webpack.config.js

-    publicPath: "https://unpkg.com/thebe@" + pkg.version + "/lib/",
+    publicPath: process.env.NODE_PREPUBLISH
+      ? "_static/lib/"
+      : "https://unpkg.com/thebe@" + pkg.version + "/lib/",


publicPath is used by webpack to load the thebe's chunked js package. Previously a local build would have been loading remaining chunks from the last version on unpkg.

Now we always set this path to use a local thebe build, both in local development and on RTD. When publishing the package to npm this should revert to the previous unpkg path which is correct for official releases.

stevejpurves · 2021-10-12T14:11:39Z

package.json

    "fmt": "prettier --trailing-comma=es5 --write *.js src/*.js examples/**/*.js test/*.js",
    "serve": "http-server -c-1 -a 127.0.0.1 -o development/binder.html",
    "serve:local": "http-server -c-1 -a 127.0.0.1 -o development/local.html",
+    "serve:examples": "node setupExamples.cjs; http-server -c-1 -a 127.0.0.1 -o examples/index.html",


added a serve command to make html examples useful in local development

Co-authored-by: Chris Holdgraf <[email protected]>

…hebe into mnt/docs-use-latest-clean

stevejpurves · 2021-10-12T14:59:45Z

@choldgraf there is still a lot to be desired about the build process here. It's complicated by trying to build for the examples folder, the docs and RTD consistently. This is working but there are some parts that could be improved for sure.

choldgraf · 2021-10-12T16:48:35Z

sounds good - just a quick thought, could we deprecate one of the build processes if it is somewhat redundant? E.g., could we:

put the examples/ folder inside of docs/
serve that folder as a static asset with our docs (so HTML pages are just direct copied through)
have them re-use the same thebe bundle that is packaged for use with docs/

Would that simplify?

stevejpurves · 2021-10-19T09:49:42Z

ok we are good to go on this one now!
I'd like to get this one in, bring in dependabots, test and cut a new release

choldgraf

This in general looks good to me (one minor suggested change and I'm +1 once those are resolved)! How long has it been since we cut a release? Maybe we should release, then merge this, and have a bit of time to see if there are regressions?

choldgraf · 2021-10-12T16:45:06Z

docs/contribute.md

+make js-dev
+```
+
+for a development build.


Could you explain here what the difference is? When would somebody use a development build vs. just make js?

have moved that line and added a note @choldgraf

choldgraf · 2021-10-19T21:40:16Z

docs/conf.py

+if os.environ.get('READ_THE_DOCS'):
+    node_modules_bin = f'{path_root}/node_modules/.bin/';
+    os.environ['PATH'] = f'{node_modules_bin}:' + os.environ["PATH"]
+    run(["npm", "install", "yarn", "jsdoc"], cwd=path_root)


just as a point of info - nox supports conda environments so you can use it with JS-style build systems, which is pretty nice. But I don't think it needs to block this! For example: https://github.com/pydata/pydata-sphinx-theme/blob/master/noxfile.py

docs/contribute.md

choldgraf · 2021-10-22T03:43:48Z

Ping for @stevejpurves - I see you've still got this in "needs review", are you hoping for 👀 from others on the team?

Co-authored-by: Chris Holdgraf <[email protected]>

stevejpurves · 2021-10-25T08:34:24Z

@choldgraf just checking back on the master branch and this PR in addition to improving the doc build process, also contains a number of improvements and a few small fixes to the dos themselves. With a few apparent errors on there and the fact I have to change the docs to the local build manually to be able to test anyways....(!) means I think it's best to get this in and then cut a release.

The changes to the library itself are really limited, it's mostly about build and the docs, so risk of regressions should be minimal. Happy to jump onto testing of a release straight away once it's up on unpkg.

I'm going to bring this in and proceed on that basis, we can always cut a release from the previous commit is we decide that's really needed.

humitos · 2021-10-25T08:39:00Z

I'm happy that this got merged! 🎉 Let me know if everything continues going well on Read the Docs or if you have any other feedback! 💪🏼

stevejpurves mentioned this pull request Sep 7, 2021

Upgrade npm and node version readthedocs/readthedocs-docker-images#107

Closed

stevejpurves changed the title ~~[MNT,DOC] use latest code in builds of the docs with RTD~~ [MNT,DOC] use latest code in builds of the docs with RTD - DRAFT Sep 7, 2021

stevejpurves mentioned this pull request Sep 7, 2021

[DOC] use the latest code in builds of the docs via GHA build #469

Closed

2 tasks

stevejpurves mentioned this pull request Sep 20, 2021

Kernel Status #459

Merged

8 tasks

stevejpurves self-assigned this Oct 4, 2021

stevejpurves force-pushed the mnt/docs-use-latest-clean branch from 4555307 to 51f0c8f Compare October 4, 2021 13:28

stevejpurves mentioned this pull request Oct 4, 2021

using local thebe in docs #298

Closed

stevejpurves added 15 commits October 4, 2021 14:52

updated makefile, using latest locally, updated conf.py for rtd build

51c6460

updated rtd config, better local vs rtd build commands in conf, some …

6a31619

…version bumps

correct os version

84e3187

tools not languages in rtd options

d682905

do path update first

39c0673

test just dep install

f0c2a6f

added yarn install

b82810a

skip prepare step

d8ed10b

explicit run script

2fad7cf

direct prod build and debugging

299e064

debug

9b47eb8

debugging jsdoc missing

01d7fa4

debug jsdoc

ddd96f0

no frozen-lockfile

d090f2d

allow yarn and jsdoc to save

0b76714

stevejpurves requested a review from choldgraf October 4, 2021 16:05

stevejpurves changed the title ~~[MNT,DOC] use latest code in builds of the docs with RTD - DRAFT~~ [MNT,DOC] use latest code in builds of the docs with RTD Oct 5, 2021

stevejpurves requested a review from moorepants October 5, 2021 10:47

choldgraf reviewed Oct 6, 2021

View reviewed changes

single build step across envs, fixed publicPath issues

8e33621

stevejpurves commented Oct 12, 2021

View reviewed changes

stevejpurves and others added 3 commits October 12, 2021 15:18

Update docs/conf.py

747b647

Co-authored-by: Chris Holdgraf <[email protected]>

removed comments for individual html files and updated docs

c228737

Merge branch 'mnt/docs-use-latest-clean' of github.com:stevejpurves/t…

ea67c14

…hebe into mnt/docs-use-latest-clean

stevejpurves added 2 commits October 18, 2021 10:07

moved examples into docs, removed redundant build steps

90eb65e

fix to webpack path for local dev

c75ccd9

stevejpurves linked an issue Oct 18, 2021 that may be closed by this pull request

Use the latest thebe.js in our documentation #285

Closed

stevejpurves mentioned this pull request Oct 18, 2021

Finish the rename + migration #230

Open

6 tasks

choldgraf approved these changes Oct 19, 2021

View reviewed changes

stevejpurves and others added 2 commits October 25, 2021 08:58

Update docs/contribute.md

04f77fd

Co-authored-by: Chris Holdgraf <[email protected]>

added note on development build

061e7e8

stevejpurves merged commit 65800aa into jupyter-book:master Oct 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MNT,DOC] use latest code in builds of the docs with RTD #472

[MNT,DOC] use latest code in builds of the docs with RTD #472

stevejpurves commented Sep 7, 2021 •

edited

Loading

humitos commented Sep 16, 2021

humitos commented Sep 16, 2021

stevejpurves commented Sep 16, 2021 •

edited

Loading

humitos commented Sep 20, 2021

humitos commented Sep 29, 2021

choldgraf commented Oct 2, 2021

stevejpurves commented Oct 4, 2021 •

edited

Loading

choldgraf left a comment

choldgraf Oct 6, 2021

stevejpurves Oct 12, 2021

choldgraf Oct 6, 2021

stevejpurves Oct 12, 2021

choldgraf Oct 19, 2021

choldgraf Oct 6, 2021

stevejpurves Oct 12, 2021

choldgraf Oct 6, 2021

choldgraf commented Oct 12, 2021

stevejpurves Oct 12, 2021

stevejpurves Oct 12, 2021 •

edited

Loading

stevejpurves Oct 12, 2021

stevejpurves Oct 12, 2021

stevejpurves commented Oct 12, 2021

choldgraf commented Oct 12, 2021

stevejpurves commented Oct 19, 2021

choldgraf left a comment

choldgraf Oct 12, 2021

stevejpurves Oct 25, 2021

choldgraf Oct 19, 2021

choldgraf commented Oct 22, 2021

stevejpurves commented Oct 25, 2021

humitos commented Oct 25, 2021

[MNT,DOC] use latest code in builds of the docs with RTD #472

[MNT,DOC] use latest code in builds of the docs with RTD #472

Conversation

stevejpurves commented Sep 7, 2021 • edited Loading

humitos commented Sep 16, 2021

humitos commented Sep 16, 2021

Read the Docs configuration file

Sphinx configuration file

stevejpurves commented Sep 16, 2021 • edited Loading

humitos commented Sep 20, 2021

humitos commented Sep 29, 2021

choldgraf commented Oct 2, 2021

stevejpurves commented Oct 4, 2021 • edited Loading

choldgraf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

choldgraf commented Oct 12, 2021

Choose a reason for hiding this comment

stevejpurves Oct 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevejpurves commented Oct 12, 2021

choldgraf commented Oct 12, 2021

stevejpurves commented Oct 19, 2021

choldgraf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

choldgraf commented Oct 22, 2021

stevejpurves commented Oct 25, 2021

humitos commented Oct 25, 2021

stevejpurves commented Sep 7, 2021 •

edited

Loading

stevejpurves commented Sep 16, 2021 •

edited

Loading

stevejpurves commented Oct 4, 2021 •

edited

Loading

stevejpurves Oct 12, 2021 •

edited

Loading