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

Tidy up pytket.circuit API docs #1178

Merged
merged 45 commits into from
Jan 29, 2024
Merged

Tidy up pytket.circuit API docs #1178

merged 45 commits into from
Jan 29, 2024

Conversation

CalMacCQ
Copy link
Contributor

@CalMacCQ CalMacCQ commented Dec 14, 2023

Description

First pass of cleaning up the pytket.circuit documentation, still plenty more to do.

  • Rather than autopopulating the circuit class docs with all methods I've used .. automethod:: to add each Circuit method individually. This means we can move the methods we want people to use to the top of the docs (i.e. Circuit.add_gate) and move others to the bottom (i.e Circuit.add_add_pauliexpcommutingsetbox and Circuit.CX)

If anyone knows how to get control of the ordering without using automethod hundreds of times then PLEASE let me know.

  • added some notes saying that add_gate is sufficent to add append any box to a circuit rather than using Circuit.add_x_box. Also saying that using the convenices methods like Circuit.CX(0, 1) are equiavlent to using Circuit.add_gate(OpType.CX, [0, 1]) .

  • Don't clutter the pytket.circuit page by expanding the logic_exp classes

before

Screenshot 2023-12-15 at 16 39 37

after

Screenshot 2023-12-15 at 16 44 16

  • Populated the right sidebar to show the contents of each pytket module (see image). This is done globally in the docs by setting "show_toc_level": 2 in html_theme_options.

before

Screenshot 2023-12-15 at 16 31 21

after

Screenshot 2023-12-15 at 16 30 40

Checklist

  • I have performed a self-review of my code.
  • I have commented hard-to-understand parts of my code.
  • I have made corresponding changes to the public API documentation.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have updated the changelog with any user-facing changes.

@CalMacCQ CalMacCQ marked this pull request as draft December 14, 2023 14:46
@CalMacCQ CalMacCQ requested a review from ss2165 December 15, 2023 16:40
@@ -14,104 +14,104 @@ pytket.circuit

.. automodule:: pytket._tket.circuit
:members: fresh_symbol
.. autoclass:: pytket._tket.circuit.Op
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think all of this _tket stuff is replaced in the docs build anyway. I don't think theres any good reason for it to be in the rst source

# The paper size ('letterpaper' or 'a4paper').
#
# 'papersize': 'letterpaper',
# The font size ('10pt', '11pt' or '12pt').
Copy link
Contributor Author

@CalMacCQ CalMacCQ Dec 15, 2023

Choose a reason for hiding this comment

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

Don't think these config settings do anything

@@ -99,6 +100,7 @@
#

html_theme_options = {
"show_toc_level": 2,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This config setting populates the right sidebar for all of the docs.

pytket/docs/conf.py Outdated Show resolved Hide resolved
@CalMacCQ CalMacCQ marked this pull request as ready for review December 15, 2023 16:54
Copy link
Collaborator

@cqc-alec cqc-alec left a comment

Choose a reason for hiding this comment

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

Having made a start at cross-checking these circuit docs with the existing ones, I think we need a script to do it for us.

I found the following missing Circuit methods, before I stopped looking: CU1, CU3, Reset, TK2, ZZMax, add_c_not.

Could you perhaps write a script that iterates over all Circuit methods (use Circuit.__dict__.keys()) and lists any that do not occur in circuit_class.rst?

@ss2165
Copy link
Member

ss2165 commented Jan 22, 2024

Could you perhaps write a script that iterates over all Circuit methods (use Circuit.dict.keys()) and lists any that do not occur in circuit_class.rst?

# print all members of Circuit that don't start with underscore that don't occur in the docs
import inspect
import re

from pytket._tket.circuit import Circuit

# paste contents of https://github.com/CQCL/tket/blob/7b0df5cf68503944cd34e593fb094ba5aa4357f4/pytket/docs/circuit_class.rst
rst = ...
in_methods = {p[0] for p in inspect.getmembers(Circuit)}
pattern = re.compile(r"(?:\bautomethod\b|\bautoproperty\b):: (\w+)\n")
in_docs = set(pattern.findall(rst))
missing = in_methods - in_docs
print([s for s in missing if not s.startswith("_")])

Output:

['CU1', 'discarded_qubits', 'bit_readout', 'Reset', 'valid_connectivity', 'CU3', 'add_multiplexed_tensored_u2', 'symbol_substitution', 'add_conditional_barrier', 'add_c_not', 'qubit_to_bit_map', 'ZZMax', 'created_qubits', 'depth_by_type', 'is_symbolic', 'add_phasepolybox', 'name', 'substitute_named', 'TK2']

@CalMacCQ
Copy link
Contributor Author

Thanks @ss2165 you beat me to it

@CalMacCQ
Copy link
Contributor Author

CalMacCQ commented Jan 22, 2024

I've added the missing methods found by the script.

Not sure if its worth using the script as part of the CI checks or whether we should just do a one-off check.

The script seems to show depth_by_type as still missing from circuit_class.rst but its in there.

@cqc-alec
Copy link
Collaborator

I've added the missing methods found by the script.

Not sure if its worth using the script as part of the CI checks or whether we should just do a one-off check.

The script seems to show depth_by_type as still missing from circuit_class.rst but its in there.

That's very strange.

Can we check in the script to the repo, as it will be useful in future even if we don't run it on the CI?

@CalMacCQ
Copy link
Contributor Author

I've added the missing methods found by the script.
Not sure if its worth using the script as part of the CI checks or whether we should just do a one-off check.
The script seems to show depth_by_type as still missing from circuit_class.rst but its in there.

That's very strange.

Can we check in the script to the repo, as it will be useful in future even if we don't run it on the CI?

Done, added the script.

.. automethod:: to_latex_file


Convenience methods for appending circuit operations
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this should be a heading?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made it a heading


.. automethod:: depth_2q

.. automethod:: depth_by_type
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's a space at the end of this line: that's why the script wasn't detecting it.

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, fixed it


from pytket._tket.circuit import Circuit

# paste contents of https://github.com/CQCL/tket/blob/7b0df5cf68503944cd34e593fb094ba5aa4357f4/pytket/docs/circuit_class.rst
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we want this comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we put a couple of lines of commentary at the top of this file to explain what it does and why?

Copy link
Contributor Author

@CalMacCQ CalMacCQ Jan 25, 2024

Choose a reason for hiding this comment

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

Done.

@CalMacCQ CalMacCQ merged commit 3e556dc into develop Jan 29, 2024
28 checks passed
@CalMacCQ CalMacCQ deleted the docs/tidy_pytket.circuit branch January 29, 2024 11:48
@CalMacCQ CalMacCQ added the documentation Improvements or additions to documentation label Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants