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

Move style docs to qcstyle.DefaultStyle #5327

Merged
merged 20 commits into from
Jan 26, 2021

Conversation

enavarro51
Copy link
Contributor

Summary

Move 'style' docs from circuit_drawer and QuantumCircuit.draw to DefaultStyle

Details and comments

Previously the docs for the style dict were duplicated in qiskit.visualization.circuit_drawer and qiskit.circuit.QuantumCircuit.draw. Based on discussions in #5223 and #5269, this PR removes them from those locations and places them in the qiskit.visualization.qcstyle.DefautlStyle class. Links to the DefaultStyle docs are added to the previous locations.

@enavarro51 enavarro51 requested review from maddy-tod, nonhermitian and a team as code owners October 30, 2020 15:39
Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

This LGTM, but before this will work you'll have to add the DefaultStyle class to the docs. See: https://qiskit.org/documentation/contributing_to_qiskit.html#documentation-structure for more details on how the docs are constructed

@@ -1167,7 +1168,7 @@ def draw(self, output=None, scale=None, filename=None, style=None,
The search path for style json files can be specified in the user
config, for example,
``circuit_mpl_style_path = /home/user/styles:/home/user``.
See: :ref:`Style Dict Doc <style-dict-doc>` for more
See: :class:`~qiskit.visualization.qcstyle.DefaultStyle` for more
Copy link
Member

Choose a reason for hiding this comment

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

This likely won't work until we add the default style class to the documentation. Normally you would do this by adding the class to qiskit/visualization/__init__.py's autosummary in the module docstring. If you do that the path will be qiskit.visaulization.DefaultStyle.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Should be done in a1fd752.

@@ -1167,7 +1168,7 @@ def draw(self, output=None, scale=None, filename=None, style=None,
The search path for style json files can be specified in the user
config, for example,
``circuit_mpl_style_path = /home/user/styles:/home/user``.
See: :ref:`Style Dict Doc <style-dict-doc>` for more
See: :class:`~qiskit.visualization.DefaultStyle` for more
Copy link
Member

Choose a reason for hiding this comment

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

Since you added the class to the docs as ~qiskit.visualization.qcstyle.DefaultStyle the path in the docs becomes

Suggested change
See: :class:`~qiskit.visualization.DefaultStyle` for more
See: :class:`~qiskit.visualization.qcstyle.DefaultStyle` for more

which I realize is what you had before. I left my previous comment assuming we'd be re-exporting DefaultStyle and add it to the autosummary line as DefaultStyle. But doing this is fine too, so you can switch this back and the link should work fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that makes sense. Done in 807b9c9.

Copy link
Member

@1ucian0 1ucian0 left a comment

Choose a reason for hiding this comment

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

Thanks!

@mergify mergify bot merged commit 0a1a7f9 into Qiskit:master Jan 26, 2021
@kdk kdk added the Changelog: None Do not include in changelog label Mar 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants