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

BREAKING: Use Sphinx Design instead of Sphinx Panels #1442

Merged
merged 16 commits into from
May 19, 2022

Conversation

choldgraf
Copy link
Collaborator

@choldgraf choldgraf commented Aug 26, 2021

This swaps out Sphinx Design for Sphinx Panels, since Sphinx Design is the package that will be maintained moving forward. It updates some of our documentation to use the new Sphinx Design syntax and examples.

To do

  • Tests pass
  • Update our docs to use sphinx-design syntax
  • Make sure our docs look correct
  • Provide a brief migration guide somewhere

@codecov
Copy link

codecov bot commented Aug 26, 2021

Codecov Report

Merging #1442 (df50882) into master (4b09c14) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1442   +/-   ##
=======================================
  Coverage   91.26%   91.26%           
=======================================
  Files           7        7           
  Lines         687      687           
=======================================
  Hits          627      627           
  Misses         60       60           
Flag Coverage Δ
pytests 91.26% <100.00%> (ø)

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

Impacted Files Coverage Δ
jupyter_book/__init__.py 100.00% <ø> (ø)
jupyter_book/config.py 94.21% <100.00%> (+0.32%) ⬆️

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 4b09c14...df50882. Read the comment docs.

@choldgraf choldgraf self-assigned this Oct 9, 2021
@choldgraf choldgraf changed the title Front page sphinx design Use Sphinx Design instead of Sphinx Panels Apr 23, 2022
@choldgraf
Copy link
Collaborator Author

Made some more progress replacing all of the sphinx-panels usage with sphinx-design, and also updated some of our front page elements to be inspired by myst-nb. Hopefully getting close...

@chrisjsewell
Copy link
Contributor

inspired by myst-nb

Yep, learn from the experts 😉

image

I'd delete all this text, IMO its superfluous and simplicity is key

@choldgraf
Copy link
Collaborator Author

choldgraf commented May 5, 2022

@mmcky or @AakashGfude do you have any idea what's going on with this latex error? It seems related to colors somehow?

[stderr]
b"convert-im6.q16: unrecognized color `d5d5d5' @ warning/color.c/GetColorCompliance/1052.\nconvert-im6.q16: non-conforming drawing primitive definition `stroke' @ error/draw.c/RenderMVGContent/4300.\n"
[stdout]
b''
Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/sphinx/ext/imgconverter.py", line 54, in convert
    subprocess.run(args, stdout=PIPE, stderr=PIPE, check=True)
  File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/subprocess.py", line 516, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['convert', '/home/runner/work/jupyter-book/jupyter-book/docs/_build/.doctrees/images/9e5ce0ca2c44a09dfe9ec8886efb34fc638de66b/jupyter-book.svg[0]', '/home/runner/work/jupyter-book/jupyter-book/docs/_build/.doctrees/images/jupyter-book.png']' returned non-zero exit status 1.

@mmcky
Copy link
Contributor

mmcky commented May 5, 2022

@choldgraf that is a new one. It looks like Imagemagick is having issues converting an svg to the png file format. Could there be a malformed svg?

@mmcky
Copy link
Contributor

mmcky commented May 5, 2022

this library uses Inkscape which is largely reported to be a more advanced converter

https://github.com/missinglinkelectronics/sphinxcontrib-svg2pdfconverter

@choldgraf
Copy link
Collaborator Author

I don't understand why that would have changed with this PR though, did Sphinx release a new patch that introduced a breaking change or something? From the error it seems like it is the jupyter-book logo causing it:

subprocess.CalledProcessError: Command '['convert', '/home/runner/work/jupyter-book/jupyter-book/docs/_build/.doctrees/images/9e5ce0ca2c44a09dfe9ec8886efb34fc638de66b/jupyter-book.svg[0]', '/home/runner/work/jupyter-book/jupyter-book/docs/_build/.doctrees/images/jupyter-book.png']' returned non-zero exit status 1.

@mmcky
Copy link
Contributor

mmcky commented May 6, 2022

So there is no new images being generated in this PR? I will try and take a closer look at the diff today and see what might be driving this switch to sphinx-design

@mmcky
Copy link
Contributor

mmcky commented May 10, 2022

@choldgraf so this error is coming from a file called jupyter-book.svg that gets added to the .doctree folder when building the docs. I can't see where this file is coming from though. It is this image causing the issue: Does this get issued by an online platform?

jupyter-book


OK it looks like that badge is malformed and color is missing the # hex identifier.

...
opacity=".1"/></linearGradient><g stroke="#d5d5d5"><rect stroke="none" fill="#fcfcfc" x="0.5" y="0.5" width="54" 
...
height="5" stroke="#fafafa"/><path d="M60.5 6.5 l-3 3v1 l3 3" stroke="d5d5d5" fill="#fafafa"/></g><image x="5" y="3" 

but I don't see this anywhere on the html site.


@choldgraf at a dead end debugging this as I just found out xindy isn't compatible with arm64 -- the m1 strikes again.

@choldgraf
Copy link
Collaborator Author

Ah @mmcky excellent investigation - your intuition is correct, it was the GitHub badge that was causing the problem, so I've modified this PR to only include those on html builds and it now passes :-)

I think that things look ready to go, so I'll mark this as ready and see if there are any QA suggestions or improvements to make before we merge.

@choldgraf choldgraf marked this pull request as ready for review May 10, 2022 07:23
@choldgraf
Copy link
Collaborator Author

OK I'm gonna merge this one in, the docs seem OK, tests are passing, and the front page looks a little bit nicer, so I declare iterative improvement accomplished :-D

We might need to spot check some docs improvement here or there if we missed something, so please keep an eye out for anything we missed.

@choldgraf choldgraf changed the title Use Sphinx Design instead of Sphinx Panels BREAKING: Use Sphinx Design instead of Sphinx Panels May 19, 2022
@choldgraf choldgraf merged commit 5df51a9 into jupyter-book:master May 19, 2022
@choldgraf choldgraf deleted the frontpage branch May 19, 2022 12:53
@mmcky mmcky mentioned this pull request Jun 2, 2022
2 tasks
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.

3 participants