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

Add links to Function/Class signatures #2774

Closed
wants to merge 12 commits into from
Closed

Conversation

arnaucasau
Copy link
Collaborator

@arnaucasau arnaucasau commented Mar 13, 2025

This PR adds a mechanism to preserve the HTML links in a Function/Class signature when converting the API docs to markdown.

Note

This PR only regenerates qiskit-addon-aqc-tensor in order to test the new links by using ./start. The rest of the API docs will be added in a follow-up or in this same PR after the new logic gets reviewed.

@arnaucasau arnaucasau force-pushed the AC/signature-links branch 2 times, most recently from ea2fe58 to ab944b4 Compare March 13, 2025 20:34
@arnaucasau arnaucasau marked this pull request as ready for review March 17, 2025 12:18
@arnaucasau arnaucasau changed the title [Do not merge] [WIP] Add links to Function/Class signatures Add links to Function/Class signatures Mar 17, 2025
const title = node.properties.title;

// We only show the title if it's from an external link
const link = title && isExternal ? `${href} "${title}"` : `${href}`;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I decided to do this because I remember we didn't like having titles like Qiskit v1.3 (noisy git diff). I kept the external references, but we can change this to have the behavior we want.

github-merge-queue bot pushed a commit that referenced this pull request Mar 18, 2025
This PR removes the duplicated `[source]` links from
`qiskit-addon-aqc-tensor`. See
#2774 (comment)
for the motivation of this change.

The only relevant commits are
2bbe774
and
935ff95.
The rest are just doing imperceptible changes to the images, which I
left in the PR to reduce the `git diff` when regenerating again for
#2774.

The API docs were regenerated by using:

```
npm run regen-apis
```

This is the output of the command:

<details><summary>output</summary>

🚀 qiskit 0.19.6 regenerated correctly
🚀 qiskit 0.24.1 regenerated correctly
🚀 qiskit 0.25.4 regenerated correctly
🚀 qiskit 0.26.2 regenerated correctly
🚀 qiskit 0.27.0 regenerated correctly
🚀 qiskit 0.28.0 regenerated correctly
🚀 qiskit 0.29.1 regenerated correctly
🚀 qiskit 0.30.1 regenerated correctly
🚀 qiskit 0.31.0 regenerated correctly
🚀 qiskit 0.32.1 regenerated correctly
🚀 qiskit 0.33.1 regenerated correctly
🚀 qiskit 0.35.0 regenerated correctly
🚀 qiskit 0.36.0 regenerated correctly
🚀 qiskit 0.37.2 regenerated correctly
🚀 qiskit 0.38.0 regenerated correctly
🚀 qiskit 0.39.5 regenerated correctly
🚀 qiskit 0.40.0 regenerated correctly
🚀 qiskit 0.41.0 regenerated correctly
🚀 qiskit 0.42.0 regenerated correctly
🚀 qiskit 0.43.0 regenerated correctly
🚀 qiskit 0.44.0 regenerated correctly
🚀 qiskit 0.45.3 regenerated correctly
🚀 qiskit 0.46.3 regenerated correctly
🚀 qiskit 1.0.2 regenerated correctly
🚀 qiskit 1.1.2 regenerated correctly
🚀 qiskit 1.2.4 regenerated correctly
✅ qiskit 1.3.3 is already up-to-date
✅ qiskit 1.4.2 is already up-to-date
✅ qiskit 2.0.0rc2 is already up-to-date
🚀 qiskit-ibm-runtime 0.14.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.15.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.16.1 regenerated correctly
🚀 qiskit-ibm-runtime 0.17.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.18.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.19.1 regenerated correctly
🚀 qiskit-ibm-runtime 0.20.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.21.1 regenerated correctly
🚀 qiskit-ibm-runtime 0.22.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.23.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.24.1 regenerated correctly
🚀 qiskit-ibm-runtime 0.25.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.26.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.27.1 regenerated correctly
🚀 qiskit-ibm-runtime 0.28.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.29.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.30.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.31.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.32.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.33.2 regenerated correctly
✅ qiskit-ibm-runtime 0.34.0 is already up-to-date
✅ qiskit-ibm-runtime 0.35.0 is already up-to-date
✅ qiskit-ibm-runtime 0.36.1 is already up-to-date
✅ qiskit-ibm-runtime 0.37.0 is already up-to-date
🚀 qiskit-ibm-runtime 0.38.0-dev regenerated correctly
🚀 qiskit-ibm-transpiler 0.3.0 regenerated correctly
🚀 qiskit-ibm-transpiler 0.4.9 regenerated correctly
🚀 qiskit-ibm-transpiler 0.5.7 regenerated correctly
🚀 qiskit-ibm-transpiler 0.6.5 regenerated correctly
🚀 qiskit-ibm-transpiler 0.7.4 regenerated correctly
🚀 qiskit-ibm-transpiler 0.8.2 regenerated correctly
🚀 qiskit-ibm-transpiler 0.9.3 regenerated correctly
✅ qiskit-ibm-transpiler 0.10.3 is already up-to-date
🚀 qiskit-addon-aqc-tensor 0.1.2 regenerated correctly
🚀 qiskit-addon-obp 0.1.0 regenerated correctly
🚀 qiskit-addon-mpf 0.1.0 regenerated correctly
🚀 qiskit-addon-mpf 0.2.0 regenerated correctly
🚀 qiskit-addon-sqd 0.7.0 regenerated correctly
✅ qiskit-addon-sqd 0.8.1 is already up-to-date
✅ qiskit-addon-sqd 0.9.0 is already up-to-date
✅ qiskit-addon-sqd 0.10.0 is already up-to-date
✅ qiskit-addon-cutting 0.9.0 is already up-to-date
🚀 qiskit-addon-utils 0.1.0 regenerated correctly
✅ qiskit-c 2.0.0 is already up-to-date

</details>
arnaucasau added a commit that referenced this pull request Mar 18, 2025
This PR removes the duplicated `[source]` links from
`qiskit-addon-aqc-tensor`. See
#2774 (comment)
for the motivation of this change.

The only relevant commits are
2bbe774
and
935ff95.
The rest are just doing imperceptible changes to the images, which I
left in the PR to reduce the `git diff` when regenerating again for
#2774.

The API docs were regenerated by using:

```
npm run regen-apis
```

This is the output of the command:

<details><summary>output</summary>

🚀 qiskit 0.19.6 regenerated correctly
🚀 qiskit 0.24.1 regenerated correctly
🚀 qiskit 0.25.4 regenerated correctly
🚀 qiskit 0.26.2 regenerated correctly
🚀 qiskit 0.27.0 regenerated correctly
🚀 qiskit 0.28.0 regenerated correctly
🚀 qiskit 0.29.1 regenerated correctly
🚀 qiskit 0.30.1 regenerated correctly
🚀 qiskit 0.31.0 regenerated correctly
🚀 qiskit 0.32.1 regenerated correctly
🚀 qiskit 0.33.1 regenerated correctly
🚀 qiskit 0.35.0 regenerated correctly
🚀 qiskit 0.36.0 regenerated correctly
🚀 qiskit 0.37.2 regenerated correctly
🚀 qiskit 0.38.0 regenerated correctly
🚀 qiskit 0.39.5 regenerated correctly
🚀 qiskit 0.40.0 regenerated correctly
🚀 qiskit 0.41.0 regenerated correctly
🚀 qiskit 0.42.0 regenerated correctly
🚀 qiskit 0.43.0 regenerated correctly
🚀 qiskit 0.44.0 regenerated correctly
🚀 qiskit 0.45.3 regenerated correctly
🚀 qiskit 0.46.3 regenerated correctly
🚀 qiskit 1.0.2 regenerated correctly
🚀 qiskit 1.1.2 regenerated correctly
🚀 qiskit 1.2.4 regenerated correctly
✅ qiskit 1.3.3 is already up-to-date
✅ qiskit 1.4.2 is already up-to-date
✅ qiskit 2.0.0rc2 is already up-to-date
🚀 qiskit-ibm-runtime 0.14.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.15.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.16.1 regenerated correctly
🚀 qiskit-ibm-runtime 0.17.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.18.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.19.1 regenerated correctly
🚀 qiskit-ibm-runtime 0.20.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.21.1 regenerated correctly
🚀 qiskit-ibm-runtime 0.22.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.23.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.24.1 regenerated correctly
🚀 qiskit-ibm-runtime 0.25.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.26.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.27.1 regenerated correctly
🚀 qiskit-ibm-runtime 0.28.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.29.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.30.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.31.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.32.0 regenerated correctly
🚀 qiskit-ibm-runtime 0.33.2 regenerated correctly
✅ qiskit-ibm-runtime 0.34.0 is already up-to-date
✅ qiskit-ibm-runtime 0.35.0 is already up-to-date
✅ qiskit-ibm-runtime 0.36.1 is already up-to-date
✅ qiskit-ibm-runtime 0.37.0 is already up-to-date
🚀 qiskit-ibm-runtime 0.38.0-dev regenerated correctly
🚀 qiskit-ibm-transpiler 0.3.0 regenerated correctly
🚀 qiskit-ibm-transpiler 0.4.9 regenerated correctly
🚀 qiskit-ibm-transpiler 0.5.7 regenerated correctly
🚀 qiskit-ibm-transpiler 0.6.5 regenerated correctly
🚀 qiskit-ibm-transpiler 0.7.4 regenerated correctly
🚀 qiskit-ibm-transpiler 0.8.2 regenerated correctly
🚀 qiskit-ibm-transpiler 0.9.3 regenerated correctly
✅ qiskit-ibm-transpiler 0.10.3 is already up-to-date
🚀 qiskit-addon-aqc-tensor 0.1.2 regenerated correctly
🚀 qiskit-addon-obp 0.1.0 regenerated correctly
🚀 qiskit-addon-mpf 0.1.0 regenerated correctly
🚀 qiskit-addon-mpf 0.2.0 regenerated correctly
🚀 qiskit-addon-sqd 0.7.0 regenerated correctly
✅ qiskit-addon-sqd 0.8.1 is already up-to-date
✅ qiskit-addon-sqd 0.9.0 is already up-to-date
✅ qiskit-addon-sqd 0.10.0 is already up-to-date
✅ qiskit-addon-cutting 0.9.0 is already up-to-date
🚀 qiskit-addon-utils 0.1.0 regenerated correctly
✅ qiskit-c 2.0.0 is already up-to-date

</details>

Fix function IDs in C API (#2793)

Small PR to grab the ID from the function signature rather than the `id`
attribute in the C API.

Actual logic change is the first commit
(37ba4c0).

Update import statements for generate_preset_pass_manager from qiskit… (#2797)

This PR updates all import statements in the /guides/ folder from:
from qiskit.transpiler.preset_passmanagers import
generate_preset_pass_manager
to:
from qiskit.transpiler import generate_preset_pass_manager

This follows the best practice mentioned in issue #2670 after the
release of Qiskit 1.3.3.

Updates to job limits (#2768)

Closes #2736.
Waiting to merge this until gen3 is deployed on dedicated systems
(sometime this week, so aiming to merge early next week - week of March
17)
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.

This new implementation looks great. Thank you for figuring out how to reuse updateLinks.ts.

FYI I opened #2805 to teach our link checker about these links. It is not blocking this PR.

Comment on lines 445 to 446
{},
new Set<string>(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

How feasible would it be to pass these arguments, like how updateLinks does it? Otherwise, some of our URLs won't be updated properly.

(You could refactor the code so that the creation of resultsByName and itemNames happens inside normalizeUrl).

Copy link
Collaborator Author

@arnaucasau arnaucasau Mar 19, 2025

Choose a reason for hiding this comment

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

I'm not sure that's possible without refactoring the signature creation part. resultsByName and itemsName get computed from initialResults which is the result of the convertFilesToMarkdown function. The new code introduced in this PR is used in the htmlSignatureToMd function which gets called in the middle of the convertFilesToMarkdown computation and therefore we can't have access to all the initialResults.

I've been trying to work around it, by using a unified plugin to detect them in a later step (inside updateLinks), but it messes up the signatures that contain markdown syntax like _ or *.

I'm still looking into it by trying to add a placeholder in the signatures

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As we talked over Slack, I took a different approach and used placeholders to know when to update the links in updateLinks(). Change done in e3b9ff0

@@ -160,6 +167,29 @@ export function relativizeLink(link: Link): Link | undefined {
return { url: `${newPrefix}/${url}`, text: newText };
}

function normalizeLinkNode(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same logic as before

});
})
.use(remarkStringify, remarkStringifyOptions)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Restored in ba38be7

Comment on lines 450 to 451
// We only show the title if it's from an external link to make the UX
// less noisy.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Given that we transform the links in a later step, we can't remove the title here. I think we should not remove it in this PR because of the complexity thereof. Now that we use updateLinks(), this should be fixed when working on #2526

@arnaucasau arnaucasau closed this Mar 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants