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

Deduplicate headers for overloaded methods #895

Merged
merged 16 commits into from
Feb 27, 2024

Conversation

arnaucasau
Copy link
Collaborator

Closes #167

The script was creating a title for every <dt> tag inside a <dl> tag. This is almost always correct given that we only have a <dt> tag for each method. The problem with overload methods is that we have a <dt> for each @typing.overload definition.

For example, the quantumcircuit.py file overloads the definition of for_loop:

https://github.com/Qiskit/qiskit/blob/244940a10c17f1dc3d74e2665a3ae869dc3d7291/qiskit/circuit/quantumcircuit.py#L4862-L4884

This is an example of the HTML generated

<dl class="py method">
    <dt class="sig sig-object py" id="qiskit.circuit.QuantumCircuit.for_loop">
    <span class="sig-name descname"><span class="pre">for_loop</span>

    <dt class="sig sig-object py">
    <span class="sig-name descname"><span class="pre">for_loop</span>
    
    <dd><p>Create a <code class="docutils literal notranslate"><span class="pre">for</span></code> loop on this circuit.</p>

To solve the problem this PR adds an extra condition to check whether the <dt> tag has an id or not. Only the first <dt> tag will have an id, so we can use this fact to create only one <h3> for each method.

@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.

Excellent! Great work. I like the solution.

Thoughts on adding a test for these? I know it's not super obvious how to do so because we punted on adding tests for processMembersAndSetMeta.

@arnaucasau
Copy link
Collaborator Author

I think it's a good idea to add tests. What do you think about adding them as a follow-up together with more cases for the processMembersAndSetMeta function? I'm going to test the generation with runtime and provider to make sure it doesn't break anything

Copy link
Member

@frankharkins frankharkins left a comment

Choose a reason for hiding this comment

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

Neat solution and nice comment, thanks!

EDIT: Approval for after resolving Eric's comments

@arnaucasau arnaucasau added this pull request to the merge queue Feb 27, 2024
Merged via the queue into Qiskit:main with commit 01c8b59 Feb 27, 2024
5 of 6 checks passed
@arnaucasau arnaucasau deleted the AC/deduplicate-method-headers branch February 27, 2024 17:04
frankharkins pushed a commit to frankharkins/documentation that referenced this pull request Jul 22, 2024
Closes Qiskit#167

The script was creating a title for every `<dt>` tag inside a `<dl>`
tag. This is almost always correct given that we only have a `<dt>` tag
for each method. The problem with overload methods is that we have a
`<dt>` for each `@typing.overload` definition.

For example, the `quantumcircuit.py` file overloads the definition of
for_loop:


https://github.com/Qiskit/qiskit/blob/244940a10c17f1dc3d74e2665a3ae869dc3d7291/qiskit/circuit/quantumcircuit.py#L4862-L4884

This is an example of the HTML generated

```html
<dl class="py method">
    <dt class="sig sig-object py" id="qiskit.circuit.QuantumCircuit.for_loop">
    <span class="sig-name descname"><span class="pre">for_loop</span>

    <dt class="sig sig-object py">
    <span class="sig-name descname"><span class="pre">for_loop</span>
    
    <dd><p>Create a <code class="docutils literal notranslate"><span class="pre">for</span></code> loop on this circuit.</p>
```
To solve the problem this PR adds an extra condition to check whether
the `<dt>` tag has an id or not. Only the first `<dt>` tag will have an
id, so we can use this fact to create only one `<h3>` for each method.
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.

Duplicate headers for overloaded methods
4 participants