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

⬆️ UPGRADE: sphinx v3, myst-nb v0.10 #911

Merged
merged 35 commits into from
Sep 1, 2020
Merged

Conversation

chrisjsewell
Copy link
Contributor

@chrisjsewell chrisjsewell commented Aug 26, 2020

@codecov
Copy link

codecov bot commented Aug 26, 2020

Codecov Report

Merging #911 into master will increase coverage by 0.21%.
The diff coverage is 93.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #911      +/-   ##
==========================================
+ Coverage   92.41%   92.63%   +0.21%     
==========================================
  Files           8        8              
  Lines         686      760      +74     
==========================================
+ Hits          634      704      +70     
- Misses         52       56       +4     
Flag Coverage Δ
#pytests 92.63% <93.28%> (+0.21%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
jupyter_book/sphinx.py 84.21% <82.35%> (-1.51%) ⬇️
jupyter_book/commands/__init__.py 94.50% <91.12%> (-1.79%) ⬇️
jupyter_book/config.py 95.34% <95.34%> (ø)
jupyter_book/__init__.py 100.00% <100.00%> (ø)
jupyter_book/pdf.py 92.00% <100.00%> (-0.60%) ⬇️
jupyter_book/toc.py 93.14% <100.00%> (+0.07%) ⬆️
jupyter_book/utils.py 86.53% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 06b8134...266f11b. Read the comment docs.

This adds options that are parsed directly to sphinx-build: nitpick, keep-going, and freshenv.
These options have then been added to the CI testing, and build warnings resolveed.
Move package imports to inside functions, so that they are "lazy" imported when calling CLI commands.
As reported in #896, a shallow copy of DEFAULT_CONFIG lead to mutations over multiple calls.
Here we replace it with a function
When running `jb build -W -n` and a missing reference was identified,
a long, superflous error traceback was printed.
Instead we simply log a message and exit with the same status code
A JSON schma has been added to validate the _config.yml, and warn of any possible issues.
This fixes a bug identified in the mathjax overrides for amsmath
Commands should be closed, so that the don't eat a subsequent value.
@chrisjsewell
Copy link
Contributor Author

Hey @mmcky @AakashGfude you might want to start looking through this.
I've finished on the main code changes, so just need to work on the document updates.

You can look through the commit history to see what's changed.
This will be a merge commit lol, since all the commits are well defined/formatted, but also combine to achieve the overarching PR goal.

@chrisjsewell
Copy link
Contributor Author

Hey mmcky AakashGfude you might want to start looking through this.

and also @choldgraf, If you have a spare moment in between the diaper changes 😉

docs/contribute/intro.md Outdated Show resolved Hide resolved
@chrisjsewell chrisjsewell linked an issue Aug 30, 2020 that may be closed by this pull request
The sphinx-panels sphinx extension has been added to the base jupyter-book distribution, and its use documented.

Some other minor documetation updates have also been applied
@chrisjsewell
Copy link
Contributor Author

chrisjsewell commented Aug 30, 2020

Ok, this is nearly done; just the addition of https://github.com/wpilibsuite/sphinxext-rediraffe, and writing the v0.8.0 CHANGELOG

I'm looking to merge this and release v0.8.0 in the next ~24-48 hours.
So speak now or forever hold your peace!

@phaustin
Copy link
Contributor

@chrisjsewell
Copy link
Contributor Author

reminder that we can remove working-on-windows

Actually, in 0cf6f69, I have already modified this; to say that we do now support/test against Windows OS 😄 . But noted the notebook execution issue with Python 3.8 still remains

@chrisjsewell
Copy link
Contributor Author

FYI also nearly fixed the jupytext issue, so no more myst-parser version incompatabilities or need for jupytext[myst] 😄 : mwouts/jupytext#602

@choldgraf
Copy link
Collaborator

I'm +1 on giving this a merge if others are 👍 as well. This is a big one and probably better to get it in sooner than later so people have a chance to play with it and see if it breaks things. I guess we should also cut a release relatively quickly since the docs will be even more outdated w/ the latest release once it's merged?

@chrisjsewell
Copy link
Contributor Author

chrisjsewell commented Aug 30, 2020

I guess we should also cut a release relatively quickly since the docs will be even more outdated w/ the latest release once it's merged?

Well that is what I meant in #911 (comment); I will literally add the "release" commit to this PR, merge, then immediately release v0.8

@chrisjsewell
Copy link
Contributor Author

have a chance to play with it and see if it breaks things

If you want to do that, you need to do it now on this PR

@choldgraf
Copy link
Collaborator

Makes sense. Personally there's nothing I'd do other than to look at how the docs work,and they seem to be good to me. Maybe @mmcky can try this branch w the quantecon books? Perhaps @bmcfee can try with his book as well to make sure there's nothing unexpected

@chrisjsewell
Copy link
Contributor Author

Yep sounds good 👍

jupytext v0.16 now properly integrates with myst-parser>=0.12
@chrisjsewell
Copy link
Contributor Author

chrisjsewell commented Sep 1, 2020

The jupytext page is now actually written as an RMarkdown file 😄 https://deploy-preview-911--jupyter-book.netlify.app/file-types/jupytext.html

@chrisjsewell
Copy link
Contributor Author

@AakashGfude @mmcky you may wish to check out https://deploy-preview-911--jupyter-book.netlify.app/content/figures.html#supported-image-formats, which is of relevance for LaTeX builds

@mmcky
Copy link
Contributor

mmcky commented Sep 1, 2020

@AakashGfude @mmcky you may wish to check out https://deploy-preview-911--jupyter-book.netlify.app/content/figures.html#supported-image-formats, which is of relevance for LaTeX builds

thanks @chrisjsewell -- the html parsing is a nice feature as it brings it closer to compatibility with how jupyter notebooks get rendered in the notebook setting.

@chrisjsewell
Copy link
Contributor Author

chrisjsewell commented Sep 1, 2020

Now that's what I call a change log: https://deploy-preview-911--jupyter-book.netlify.app/reference/_changelog.html

@chrisjsewell chrisjsewell merged commit cccdc60 into master Sep 1, 2020
@chrisjsewell chrisjsewell deleted the upgrade/myst-nb-0.9 branch September 1, 2020 14:40
@rodrigogiraoserrao
Copy link
Contributor

rodrigogiraoserrao commented Sep 11, 2020

Well mainly I just want someone(s) else to give it a second look, to make sure I'm not going to inadvertently break everyone's existing books 😬
Personally, I think the new logic is reasonable and, at the very least, we now have a way to "programmatically document" the behaviour (via the test cases), and also a way for users to directly understand how their config is being translated (via jb config sphinx).

A bit late, but looks good! Good job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants