feat: add gate class docs to the subroutines in circuits #800
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #800 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 129 129
Lines 8519 8520 +1
Branches 1905 1905
=========================================
+ Hits 8519 8520 +1 ☔ View full report in Codecov by Sentry. |
| function_docstring = ( | ||
| "\n\n".join([cls_docstring, func.__doc__]) if len(cls_docstring) > 0 else func.__doc__ | ||
| ) | ||
| setattr(function_attr, "__doc__", function_docstring) |
There was a problem hiding this comment.
Do you whether setting __doc__ in this way allows the docstring to appear in IDEs such as VS Code?
There was a problem hiding this comment.
I don't have VSCode setup so I am unsure at the moment.
| @@ -84,13 +84,15 @@ class Circuit: | |||
| _ALL_QUBITS = "ALL" # Flag to indicate all qubits in _qubit_observable_mapping | |||
|
|
|||
| @classmethod | |||
| def register_subroutine(cls, func: SubroutineCallable) -> None: | |||
| def register_subroutine(cls, func: SubroutineCallable, cls_docstring: str) -> None: | |||
There was a problem hiding this comment.
Perhaps this should be str | None and default to None? For backward compatibility reasons it seems like it needs a default value.
| setattr(function_attr, "__doc__", func.__doc__) | ||
| # docstrings don't like a single newline for some reason. | ||
| function_docstring = ( | ||
| "\n\n".join([cls_docstring, func.__doc__]) if len(cls_docstring) > 0 else func.__doc__ |
There was a problem hiding this comment.
| "\n\n".join([cls_docstring, func.__doc__]) if len(cls_docstring) > 0 else func.__doc__ | |
| "\n\n".join([cls_docstring, func.__doc__]) if cls_docstring else func.__doc__ |
| @@ -1522,13 +1528,15 @@ def __call__(self, arg: Any | None = None, **kwargs) -> Circuit: | |||
| return self.make_bound_circuit(param_values) | |||
|
|
|||
|
|
|||
| def subroutine(register: bool = False) -> Callable: | |||
| def subroutine(register: bool = False, cls_docstring: str = "") -> Callable: | |||
There was a problem hiding this comment.
should this be str | None and default to None? Just to avoid a potential usage mistake, seems like None and "" should behave the same way here.
| @@ -90,7 +90,7 @@ def fixed_qubit_count() -> int: | |||
| return 1 | |||
|
|
|||
| @staticmethod | |||
| @circuit.subroutine(register=True) | |||
| @circuit.subroutine(register=True, cls_docstring=__doc__) | |||
There was a problem hiding this comment.
Did you mean to only add this to the h gate, or do you also want to add this to the rest of the gates?
Issue #, if available:
Description of changes:
Testing done:
Merge Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.General
Tests
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.