Conversation
Codecov Report
@@ Coverage Diff @@
## master #35 +/- ##
=======================================
Coverage 99.87% 99.87%
=======================================
Files 18 18
Lines 1559 1559
Branches 263 263
=======================================
Hits 1557 1557
Misses 1 1
Partials 1 1 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
JoelPasvolsky
left a comment
There was a problem hiding this comment.
Just a couple of comments on installing dependencies to get the JN conversions working. It took me a while but mostly because I tried and moved on from Win and Py 3.7 environments before installing a pyenv env in my Ubuntu 16.04 VM and working with Py 3.9.
Now that I have it working locally, I'll go through the rest.
There was a problem hiding this comment.
@thisac, I'm about halfway through the first demo, so I'll submit this half in case you have reservations, we can talk through before I make similar suggestions for the rest. It's too be more consistent with the writing style in the rest of our docs
Co-authored-by: Joel Pasvolsky <34041130+JoelPasvolsky@users.noreply.github.com>
Co-authored-by: Joel Pasvolsky <34041130+JoelPasvolsky@users.noreply.github.com>
docs/requirements.txt
Outdated
| sphinx_autodoc_typehints==1.19.5 | ||
| sphinx-design==0.4.1 | ||
| ipykernel==6.23.1 | ||
| matplotlib==3.71 |
There was a problem hiding this comment.
I'm on an old Ubuntu with Py3.9, so this might not be supported anyway but I get
ERROR: Could not find a version that satisfies the requirement matplotlib==3.71
There was a problem hiding this comment.
Oh, should most definitely have been 3.7.1. Thanks for the discovery.
randomir
left a comment
There was a problem hiding this comment.
A few comments on the makefile.
| # gates accept slightly different arguments, although you can _always_ pass the | ||
| # qubits as sequences via the keyword argument `qubits`. | ||
| # | ||
| # :::note |
There was a problem hiding this comment.
Not working for me as well (in the jupyter notebook).
| cp -a $(DEMOFOLDER)/. $(OUTFOLDER) | ||
|
|
||
| # 1. find all Python demos and convert them to Jupyter notebooks | ||
| find $(DEMOFOLDER) -name '*.py' -exec sh -c '$(JUPYTEXT) --to ipynb $$1 --output "$(OUTFOLDER)/$$(basename $$1 .py).ipynb"' sh {} \; |
There was a problem hiding this comment.
You can use shell's built-in parameter expansion to avoid calling (external) baseline.
| find $(OUTFOLDER) -name '*.rst' -exec sh -c 'sed --in-place \ | ||
| -e $(ADD_FILE_NAME_LABEL) \ | ||
| -e "s/\`\`Circuit\`\`/:class:\`~dwave.gate.circuit.Circuit\`/g" \ | ||
| -e "s/\`\`Circuit\.\(\w*\)\`\`/:meth:\`~dwave.gate.circuit.Circuit.\1\`/g" \ | ||
| -e "s/\`\`ParametricCircuit\`\`/:class:\`~dwave.gate.circuit.ParametricCircuit\`/g" \ | ||
| -e "s/\`\`Operation\`\`/:class:\`~dwave.gate.operations.base.Operation\`/g" \ | ||
| -e "s/\`\`operations\`\`/:mod:\`~dwave.gate.operations\`/g" \ | ||
| -e "s/\`\`ops\.\(\w*\)\`\`/:class:\`~dwave.gate.operations.operations.\1\`/g" \ | ||
| -e "s/\`\`Measurement\`\`/:mod:\`~dwave.gate.operations.base.Measurement\`/g" \ | ||
| -e "s/\`\`ParametricOperation\`\`/:mod:\`~dwave.gate.operations.base.ParametricOperation\`/g" \ | ||
| -e "s/\`\`ControlledOperation\`\`/:mod:\`~dwave.gate.operations.base.ControlledOperation\`/g" \ | ||
| -e "s/\`\`ParametricControlledOperation\`\`/:mod:\`~dwave.gate.operations.base.ParametricControlledOperation\`/g" \ | ||
| -e "s/\`\`simulate\`\`/:mod:\`~dwave.gate.simulator.simulate\`/g" \ | ||
| -e "s/\`\`simulator\.\(\w*\)\`\`/:class:\`~dwave.gate.simulator.\1\`/g" \ | ||
| -e $(ADD_DOWNLOAD_BUTTONS) \ | ||
| $$1' sh {} \; \ |
There was a problem hiding this comment.
If you prefer sed, I'd extract these substitution commands in a stand-alone sed script, just to avoid the escape hell.
| @$(SPHINXBUILD) -M help "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O) | ||
|
|
||
| .PHONY: demos | ||
| demos: |
There was a problem hiding this comment.
This build rule is sufficiently complex to warrant a stand-alone shell script.
Or, given how shell scripts are generally fragile, unmaintainable and unscalable, perhaps switch to something more portable and extensible, like Python? 🙂
There was a problem hiding this comment.
Thought about it, and it's a good idea. I don't necessarily mind the simplicity of having it like this though. Perhaps leaving it like this for the time being and potentially reevaluate later.
| find $(DEMOFOLDER) -name '*.py' -exec sh -c '$(JUPYTEXT) --to ipynb $$1 --output "$(OUTFOLDER)/$$(basename $$1 .py).ipynb"' sh {} \; | ||
|
|
||
| # 2. find all Jupyter notebooks and convert them into Sphinx rst files | ||
| find $(OUTFOLDER) -name '*.ipynb' -exec $(NBCONVERT) {} --to rst --output-dir=$(OUTFOLDER) --execute --RegexRemovePreprocessor.patterns="^%" \; |
There was a problem hiding this comment.
Maybe you'll want to skip processing notebooks under .ipynb_checkpoints/.
| :download:\`Download as Jupyter Notebook \<$$(basename $$1 .rst).ipynb\>\`\n" | ||
|
|
||
| # Script to add file names as labels to the top of the rst files | ||
| ADD_FILE_NAME_LABEL = "1s/^/.. _$$(basename $$1 .rst):\n\n/" |
There was a problem hiding this comment.
You can also use sed's insert command to prepend text (slightly simpler expression). When combined with parameter expansion switch as suggested below:
| ADD_FILE_NAME_LABEL = "1s/^/.. _$$(basename $$1 .rst):\n\n/" | |
| ADD_FILE_NAME_LABEL = "1i .. _$${1%.rst}:\n" |
Adds a demos page to the documentation, including a beginner's tutorial on how to use
dwave-gate(taken from the examples folder). The demos are stored as Python notebooks in thedocs/demos/demosfolder and converted to Sphinx rst files and Jupyter notebooks using nbconvert and jupytext.To convert a notebook into a valid Python notebook, run
jupytext --to py:sphinx name_of_notebook_file.ipynb.Note: Requires Pandoc to build the correct files, along with Python packages
jupytextandnbconvert(already in doc requirements), which might require some changes to the SDK build.