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

Fix API generation regression #899

Merged
merged 28 commits into from
Feb 28, 2024

Conversation

arnaucasau
Copy link
Collaborator

@arnaucasau arnaucasau commented Feb 27, 2024

Closes #224

This PR fixes an issue with the API generation script that causes regressions in attributes. For each attribute, we show on the documentation website, its type (if exists) and its default value (if exists).

An example of an attribute could be instance: str | None = None which has the type str | None and the default value None. This is its representation on the website:

Captura desde 2024-02-27 19-12-42

To find the attribute and the default value, the script searches for the index of the colon (type separator) and the index of the equal sign (default value separator). The problem is that those two values are optional, and we could have, for example, a default value involving a string containing a colon symbol that would be detected as the type separator when it's not.

To fix this, we could only search for the type separator before the default value. We don't have any problem with the equal sign because it cannot be a type of attribute.

The PR has regenerated all versions of qiskit, runtime, and provider.

@arnaucasau arnaucasau changed the title [WIP] Fix API generation regression Fix API generation regression Feb 28, 2024
@arnaucasau arnaucasau marked this pull request as ready for review February 28, 2024 10:58
@qiskit-bot
Copy link
Contributor

Thanks for contributing to Qiskit documentation!

Before your PR can be merged, it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. Thanks! 🙌

Copy link
Collaborator

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Nice work!

This would also be great to test, but totally fine to do in a follow up.

Comment on lines -234 to 236
### DEFAULT\_CONFIGURATION = \{'backend\_name'

`= {'backend_name':`
### DEFAULT\_CONFIGURATION

`= {'backend_name': 'qasm_simulator', 'backend_version': '2.1.0', 'basis_gates': ['u1', 'u2', 'u3', 'rz', 'sx', 'x', 'cx', 'id', 'unitary'], 'conditional': True, 'coupling_map': None, 'description': 'A python simulator for qasm experiments', 'gates': [{'name': 'u1', 'parameters': ['lambda'], 'qasm_def': 'gate u1(lambda) q { U(0,0,lambda) q; }'}, {'name': 'u2', 'parameters': ['phi', 'lambda'], 'qasm_def': 'gate u2(phi,lambda) q { U(pi/2,phi,lambda) q; }'}, {'name': 'u3', 'parameters': ['theta', 'phi', 'lambda'], 'qasm_def': 'gate u3(theta,phi,lambda) q { U(theta,phi,lambda) q; }'}, {'name': 'rz', 'parameters': ['phi'], 'qasm_def': 'gate rz(phi) q { U(0,0,phi) q; }'}, {'name': 'sx', 'parameters': [], 'qasm_def': 'gate sx(phi) q { U(pi/2,7*pi/2,pi/2) q; }'}, {'name': 'x', 'parameters': [], 'qasm_def': 'gate x q { U(pi,7*pi/2,pi/2) q; }'}, {'name': 'cx', 'parameters': [], 'qasm_def': 'gate cx c,t { CX c,t; }'}, {'name': 'id', 'parameters': [], 'qasm_def': 'gate id a { U(0,0,0) a; }'}, {'name': 'unitary', 'parameters': ['matrix'], 'qasm_def': 'unitary(matrix) q1, q2,...'}], 'local': True, 'max_shots': 65536, 'memory': True, 'n_qubits': 24, 'open_pulse': False, 'simulator': True, 'url': 'https://github.com/Qiskit/qiskit-terra'}`
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was wondering if we should go back to the qiskit.org style of the default value being on the same line as the attribute. But this example makes it clear that that won't work. The default value and type hint might be way too long.

@@ -216,9 +216,7 @@ Expanded Pass manager stages including `pre_` and `post_` phases.

<span id="qiskit.transpiler.StagedPassManager.invalid_stage_regex" />

### invalid\_stage\_regex = re.compile('\\\s|\\\\+|\\\\-|\\\\\*|\\\\/|\\\\\\\\|\\\\%|\\\\\<|\\\\>|\\\\@|\\\\!|\\\\\~|\\\\^|\\\\&|\\\\
Copy link
Collaborator

Choose a reason for hiding this comment

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

!!!!

@arnaucasau arnaucasau added this pull request to the merge queue Feb 28, 2024
Merged via the queue into Qiskit:main with commit 8158062 Feb 28, 2024
5 of 6 checks passed
@arnaucasau arnaucasau deleted the AC/fix-attribute-regression branch February 28, 2024 15:55
frankharkins pushed a commit to frankharkins/documentation that referenced this pull request Jul 22, 2024
Closes Qiskit#224

This PR fixes an issue with the API generation script that causes
regressions in attributes. For each attribute, we show on the
documentation website, its type (if exists) and its default value (if
exists).

An example of an attribute could be `instance: str | None = None` which
has the type `str | None` and the default value `None`. This is its
representation on the website:

![Captura desde 2024-02-27
19-12-42](https://github.com/Qiskit/documentation/assets/47946624/e2cce336-9d0a-4da7-bd80-3f7d7cf1f6c4)

To find the attribute and the default value, the script searches for the
index of the colon (type separator) and the index of the equal sign
(default value separator). The problem is that those two values are
optional, and we could have, for example, a default value involving a
string containing a colon symbol that would be detected as the type
separator when it's not.

To fix this, we could only search for the type separator before the
default value. We don't have any problem with the equal sign because it
cannot be a type of attribute.

The PR has regenerated all versions of qiskit, runtime, and provider.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

API generation regression in properties
4 participants