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.

Sign up for GitHub

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

Jump to bottom

Regression in EquivalenceLibrary documentation from oxidation #13258

Open
Eric-Arellano opened this issue Oct 2, 2024 · 3 comments · May be fixed by #13291
Open

Regression in EquivalenceLibrary documentation from oxidation #13258

Eric-Arellano opened this issue Oct 2, 2024 · 3 comments · May be fixed by #13291
Assignees
@raynelfss
Labels
bug Something isn't working documentation Something is not clear or an error documentation
Milestone
1.3.0

Comments

@Eric-Arellano
Copy link
Collaborator

When generating the 1.3 dev docs, I noticed this diff:

diff 
diff --git a/docs/api/qiskit/dev/qiskit.circuit.EquivalenceLibrary.mdx b/docs/api/qiskit/dev/qiskit.circuit.EquivalenceLibrary.mdx
index 9d46ec616e..8cbc564e19 100644
--- a/docs/api/qiskit/dev/qiskit.circuit.EquivalenceLibrary.mdx
+++ b/docs/api/qiskit/dev/qiskit.circuit.EquivalenceLibrary.mdx
@@ -8,40 +8,22 @@ python_api_name: qiskit.circuit.EquivalenceLibrary
 
 # EquivalenceLibrary
 
-<Class id="qiskit.circuit.EquivalenceLibrary" isDedicatedPage={true} github="https:/Qiskit/qiskit/tree/main/qiskit/circuit/equivalence.py#L32-L267" signature="qiskit.circuit.EquivalenceLibrary(*, base=None)" modifiers="class">
-  Bases: [`object`](https://docs.python.org/3/library/functions.html#object "(in Python v3.12)")
+<Class id="qiskit.circuit.EquivalenceLibrary" isDedicatedPage={true} github="https:/Qiskit/qiskit/tree/main/qiskit/circuit/equivalence.py#L29-L94" signature="qiskit.circuit.EquivalenceLibrary" modifiers="class">
+  Bases: `BaseEquivalenceLibrary`
 
   A library providing a one-way mapping of Gates to their equivalent implementations as QuantumCircuits.
 
-  Create a new equivalence library.
-
-  **Parameters**
-
-  **base** (*Optional\[*[*EquivalenceLibrary*](#qiskit.circuit.EquivalenceLibrary "qiskit.circuit.EquivalenceLibrary")*]*) – Base equivalence library to be referenced if an entry is not found in this library.
-
   ## Attributes
 
   ### graph
 
-  <Attribute id="qiskit.circuit.EquivalenceLibrary.graph">
-    Return graph representing the equivalence library data.
-
-    This property should be treated as read-only as it provides a reference to the internal state of the [`EquivalenceLibrary`](#qiskit.circuit.EquivalenceLibrary "qiskit.circuit.EquivalenceLibrary") object. If the graph returned by this property is mutated it could corrupt the the contents of the object. If you need to modify the output `PyDiGraph` be sure to make a copy prior to any modification.
-
-    **Returns**
-
-    A graph object with equivalence data in each node.
-
-    **Return type**
-
-    PyDiGraph
-  </Attribute>
+  <Attribute id="qiskit.circuit.EquivalenceLibrary.graph" />
 
   ## Methods
 
   ### add\_equivalence
 
-  <Function id="qiskit.circuit.EquivalenceLibrary.add_equivalence" github="https:/Qiskit/qiskit/tree/main/qiskit/circuit/equivalence.py#L76-L113" signature="add_equivalence(gate, equivalent_circuit)">
+  <Function id="qiskit.circuit.EquivalenceLibrary.add_equivalence" signature="add_equivalence(gate, equivalent_circuit)">
     Add a new equivalence to the library. Future queries for the Gate will include the given circuit, in addition to all existing equivalences (including those from base).
 
     Parameterized Gates (those including qiskit.circuit.Parameters in their Gate.params) can be marked equivalent to parameterized circuits, provided the parameters match.
@@ -54,7 +36,7 @@ python_api_name: qiskit.circuit.EquivalenceLibrary
 
   ### draw
 
-  <Function id="qiskit.circuit.EquivalenceLibrary.draw" github="https:/Qiskit/qiskit/tree/main/qiskit/circuit/equivalence.py#L199-L224" signature="draw(filename=None)">
+  <Function id="qiskit.circuit.EquivalenceLibrary.draw" github="https:/Qiskit/qiskit/tree/main/qiskit/circuit/equivalence.py#L33-L58" signature="draw(filename=None)">
     Draws the equivalence relations available in the library.
 
     **Parameters**
@@ -78,7 +60,7 @@ python_api_name: qiskit.circuit.EquivalenceLibrary
 
   ### get\_entry
 
-  <Function id="qiskit.circuit.EquivalenceLibrary.get_entry" github="https:/Qiskit/qiskit/tree/main/qiskit/circuit/equivalence.py#L156-L178" signature="get_entry(gate)">
+  <Function id="qiskit.circuit.EquivalenceLibrary.get_entry" signature="get_entry(gate)">
     Gets the set of QuantumCircuits circuits from the library which equivalently implement the given Gate.
 
     Parameterized circuits will have their parameters replaced with the corresponding entries from Gate.params.
@@ -102,7 +84,7 @@ python_api_name: qiskit.circuit.EquivalenceLibrary
 
   ### has\_entry
 
-  <Function id="qiskit.circuit.EquivalenceLibrary.has_entry" github="https:/Qiskit/qiskit/tree/main/qiskit/circuit/equivalence.py#L115-L127" signature="has_entry(gate)">
+  <Function id="qiskit.circuit.EquivalenceLibrary.has_entry" signature="has_entry(gate)">
     Check if a library contains any decompositions for gate.
 
     **Parameters**
@@ -122,39 +104,15 @@ python_api_name: qiskit.circuit.EquivalenceLibrary
 
   ### keys
 
-  <Function id="qiskit.circuit.EquivalenceLibrary.keys" github="https:/Qiskit/qiskit/tree/main/qiskit/circuit/equivalence.py#L180-L186" signature="keys()">
-    Return list of keys to key to node index map.
-
-    **Returns**
-
-    Keys to the key to node index map.
-
-    **Return type**
-
-    List
-  </Function>
+  <Function id="qiskit.circuit.EquivalenceLibrary.keys" signature="keys()" />
 
   ### node\_index
 
-  <Function id="qiskit.circuit.EquivalenceLibrary.node_index" github="https:/Qiskit/qiskit/tree/main/qiskit/circuit/equivalence.py#L188-L197" signature="node_index(key)">
-    Return node index for a given key.
-
-    **Parameters**
-
-    **key** (*Key*) – Key to an equivalence.
-
-    **Returns**
-
-    Index to the node in the graph for the given key.
-
-    **Return type**
-
-    Int
-  </Function>
+  <Function id="qiskit.circuit.EquivalenceLibrary.node_index" signature="node_index(key)" />
 
   ### set\_entry
 
-  <Function id="qiskit.circuit.EquivalenceLibrary.set_entry" github="https:/Qiskit/qiskit/tree/main/qiskit/circuit/equivalence.py#L129-L154" signature="set_entry(gate, entry)">
+  <Function id="qiskit.circuit.EquivalenceLibrary.set_entry" signature="set_entry(gate, entry)">
     Set the equivalence record for a Gate. Future queries for the Gate will return only the circuits provided.
 
     Parameterized Gates (those including qiskit.circuit.Parameters in their Gate.params) can be marked equivalent to parameterized circuits, provided the parameters match.

Losing the GitHub source code link doesn't seem very feasible to fix because we populate it based on inspect.getsourcefile. Imo it's not that big of a deal because the class still has the link, and it might confuse users to point them to Rust code.

The bigger concern is the type hints, parameters, and description.
@Eric-Arellano Eric-Arellano added documentation Something is not clear or an error documentation bug Something isn't working labels Oct 2, 2024
@Eric-Arellano Eric-Arellano added this to the 1.3.0 milestone Oct 2, 2024
@mtreinish
Copy link
Member

Yeah we should definitely fix this, the simple first step is to just add back the deleted docstrings from the python version to the pyo3 struct and methods.

However, the type hints are a bit more involved since there isn't a C api method of returning type hints from python. The best we can do is add a stub file like suggested in #12803 although it'd be incomplete to start as adding a complete one is a much larger undertaking (it's something we did in rustworkx and took the better part of a year given the api surface). I guess the easier first step would be to add them to the docstring so the info isn't lost even if means a regression from the mypy perspective.

@Eric-Arellano
Copy link
Collaborator Author

The best we can do is add a stub file like suggested in #12803 although it'd be incomplete to start as adding a complete one is a much larger undertaking (it's something we did in rustworkx and took the better part of a year given the api surface).

That's fine to be incomplete to start. I'd encourage taking that incremental approach. Type hints are helpful to developers in day-to-day because of the close IDE integrations, so it'd be a bummer to lose type information.

One benefit of adding the stub file—even if it's quite barren—is it makes it easier for all new oxidation to do the right thing. You can stop the problem from getting even bigger.

@Eric-Arellano Eric-Arellano mentioned this issue Oct 2, 2024
Merged
@jakelishman
Copy link
Member

Fwiw, the original Python didn't have type hints either. What you can see is just part of the docstring.

github-merge-queue bot pushed a commit to Qiskit/documentation that referenced this issue Oct 2, 2024
@Eric-Arellano
Regen dev docs (#2071)
d00405d 
Generated with `npx tsx scripts/js/commands/api/syncDevDocs.ts` (cron
job currently not working).

Note that Runtime removes all v1 Fake Provider pages. We'll set up
redirects once this version becomes stable.

There is also a Qiskit SDK regression tracked by
Qiskit/qiskit#13258.
@raynelfss raynelfss linked a pull request Oct 7, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@raynelfss raynelfss

Labels
bug Something isn't working documentation Something is not clear or an error documentation
Projects
None yet
Milestone
1.3.0
Development

Successfully merging a pull request may close this issue.

Add missing docstrings for EquivalenceLibrary raynelfss/qiskit
4 participants
@mtreinish @jakelishman @Eric-Arellano @raynelfss