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

Redesign documentation #78

Merged
merged 22 commits into from
Apr 7, 2020
Merged

Redesign documentation #78

merged 22 commits into from
Apr 7, 2020

Conversation

mariaschuld
Copy link
Contributor

@mariaschuld mariaschuld commented Apr 4, 2020

This is a major re-design of the plugin's documentation, integrating it into the pennylane.ai webpage. It is best reviewed by building the docs locally, since so many files are changed.

I propose this format for all plugins, and it should therefore be regarded as a potential template for plugin docs (for those plugins hosted by Xanadu).

Possible issues:
I copied the xanadu theme over to here, but I am not 100% familiar with what is needed from the theme.

@codecov
Copy link

codecov bot commented Apr 4, 2020

Codecov Report

Merging #78 into master will not change coverage by %.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #78   +/-   ##
=======================================
  Coverage   99.11%   99.11%           
=======================================
  Files           7        7           
  Lines         340      340           
=======================================
  Hits          337      337           
  Misses          3        3           
Impacted Files Coverage Δ
pennylane_qiskit/aer.py 100.00% <ø> (ø)
pennylane_qiskit/basic_aer.py 100.00% <ø> (ø)
pennylane_qiskit/converter.py 100.00% <ø> (ø)
pennylane_qiskit/ibmq.py 100.00% <ø> (ø)
pennylane_qiskit/qiskit_device.py 98.62% <ø> (ø)

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 269802a...b8c5e20. Read the comment docs.

@mariaschuld mariaschuld self-assigned this Apr 4, 2020
@mariaschuld mariaschuld requested review from antalszava and josh146 and removed request for antalszava and josh146 April 4, 2020 13:58
@mariaschuld
Copy link
Contributor Author

The failing test may be related to qiskit server problems, see Issue from 11 hours ago: Qiskit/qiskit-ibmq-provider#617

README.rst Show resolved Hide resolved
README.rst Show resolved Hide resolved
README.rst Show resolved Hide resolved
doc/about.rst Outdated Show resolved Hide resolved
doc/conf.py Outdated Show resolved Hide resolved
doc/devices/aer.rst Show resolved Hide resolved
doc/devices/aer.rst Show resolved Hide resolved
doc/devices/aer.rst Show resolved Hide resolved
doc/devices/aer.rst Outdated Show resolved Hide resolved
Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

doc/devices/aer.rst Outdated Show resolved Hide resolved
doc/devices/aer.rst Outdated Show resolved Hide resolved
doc/devices/basicaer.rst Outdated Show resolved Hide resolved
doc/devices/basicaer.rst Outdated Show resolved Hide resolved
doc/devices/ibmq.rst Show resolved Hide resolved
doc/devices/overview.rst Show resolved Hide resolved
doc/devices/overview.rst Outdated Show resolved Hide resolved
doc/devices/overview.rst Show resolved Hide resolved
@mariaschuld
Copy link
Contributor Author

Thanks @josh146 for your great contributions.

Let's see what @antalszava thinks about the new layout.

@josh146
Copy link
Member

josh146 commented Apr 6, 2020

Something that has crossed my mind: the API part of the documentation still uses the old style documentation (entire module on one page) as opposed to the new-style (one page per class).

This should be relatively easy, and might make it more maintainable:

image

The git diff is just:

diff --git a/doc/code/pennylane_qiskit.rst b/doc/code/pennylane_qiskit.rst
new file mode 100644
index 0000000..d1b9aab
--- /dev/null
+++ b/doc/code/pennylane_qiskit.rst
@@ -0,0 +1,8 @@
+pennylane-qiskit
+================
+
+.. currentmodule:: pennylane_qiskit
+
+.. automodapi:: pennylane_qiskit
+    :no-heading:
+    :include-all-objects:
diff --git a/doc/conf.py b/doc/conf.py
index d900e7c..2d4d698 100644
--- a/doc/conf.py
+++ b/doc/conf.py
@@ -40,9 +40,15 @@ extensions = [
     'sphinx.ext.inheritance_diagram',
     "sphinx.ext.intersphinx",
     'sphinx.ext.viewcode',
+    "sphinx_automodapi.automodapi",
     'edit_on_github'
 ]

+autosummary_generate = True
+autosummary_imported_members = False
+automodapi_toctreedirnm = "code/api"
+automodsumm_inherited_members = True
+
 # Add any paths that contain templates here, relative to this directory.
 templates_path = ['_templates', 'xanadu_theme']

diff --git a/doc/index.rst b/doc/index.rst
index 0ee9e29..046768f 100644
--- a/doc/index.rst
+++ b/doc/index.rst
@@ -44,7 +44,4 @@ PennyLane-Qiskit Plugin
    :caption: API
    :hidden:

-   code/qiskit_device
-   code/basic_aer
-   code/aer
-   code/ibmq
+   code/pennylane_qiskit
diff --git a/doc/requirements.txt b/doc/requirements.txt
index 30aa0f4..5fd9be7 100644
--- a/doc/requirements.txt
+++ b/doc/requirements.txt
@@ -6,3 +6,4 @@ pybind11
 qiskit
 git+https://github.com/XanaduAI/pennylane.git#egg=pennylane
 dc-qiskit-algorithms
+sphinx-automodapi

The downside is that everything will be documented, and we have less control over the look, what is included, etc. So I'm on the fence with this.

@mariaschuld
Copy link
Contributor Author

I'm in favour - the API is really only useful for developers, since users never really instantiate a class (they use the device). So it makes sense to document everything public.

Can you just push the change?

Maybe we should add a comment to the module docstring saying that the classes are automatically created when calling qml.device('qiskit.XXX',...)?

@josh146
Copy link
Member

josh146 commented Apr 6, 2020

Can you just push the change?

Done in 4e52c12 :)

Makefile Show resolved Hide resolved
doc/devices/aer.rst Outdated Show resolved Hide resolved
doc/devices/basicaer.rst Outdated Show resolved Hide resolved
README.rst Show resolved Hide resolved
doc/index.rst Outdated Show resolved Hide resolved
doc/conf.py Show resolved Hide resolved
Copy link
Contributor

@antalszava antalszava left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mariaschuld this is awesome, thank you so much! 💯 😊 Looks really neat! I had a couple of suggestions & questions, though none of them was major

@mariaschuld mariaschuld merged commit a43c07b into master Apr 7, 2020
@mariaschuld mariaschuld deleted the redesign_docs branch April 7, 2020 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants