search

Qiskit / qiskit

Qiskit is an open-source SDK for working with quantum computers at the level of extended quantum circuits, operators, and primitives.
https://www.ibm.com/quantum/qiskit
Apache License 2.0
5.28k stars 2.37k forks source link

Regression in EquivalenceLibrary documentation from oxidation #13258

Closed Eric-Arellano closed 1 week ago

Eric-Arellano commented 1 month ago

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

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 - - Bases: [`object`](https://docs.python.org/3/library/functions.html#object "(in Python v3.12)") + + 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 - - 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 - + ## Methods ### add\_equivalence - + 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 - + Draws the equivalence relations available in the library. **Parameters** @@ -78,7 +60,7 @@ python_api_name: qiskit.circuit.EquivalenceLibrary ### get\_entry - + 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 - + Check if a library contains any decompositions for gate. **Parameters** @@ -122,39 +104,15 @@ python_api_name: qiskit.circuit.EquivalenceLibrary ### keys - - Return list of keys to key to node index map. - - **Returns** - - Keys to the key to node index map. - - **Return type** - - List - + ### node\_index - - 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 - + ### set\_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.

mtreinish commented 1 month ago

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 commented 1 month ago

The best we can do is add a stub file like suggested in https://github.com/Qiskit/qiskit/issues/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.

jakelishman commented 1 month ago

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