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 accidental deletion of GitHub source links for methods #940

Merged
merged 19 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions scripts/lib/api/htmlToMd.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ Can be either (1) a dictionary mapping XX angle values to fidelity at that angle

<span id="qiskit_ibm_runtime.Estimator.run" />

\`Estimator.run(circuits, observables, parameter_values=None, **kwargs)\` [GitHub](https://github.com/Qiskit/qiskit-ibm-runtime/tree/0.9.2/qiskit_ibm_runtime/estimator.py "view source code")
\`Estimator.run(circuits, observables, parameter_values=None, **kwargs)\`

Submit a request to the estimator primitive program.
",
Expand Down Expand Up @@ -855,7 +855,7 @@ By default this is sys.stdout.</p></li>

<span id="qiskit_ibm_provider.job.IBMCircuitJob.wait_for_final_state" />

\`IBMCircuitJob.wait_for_final_state(timeout=None) \`[GitHub](https://github.com/Qiskit/qiskit-ibm-runtime/tree/0.9.2/qiskit_ibm_provider/job/ibm_circuit_job.py "view source code")
\`IBMCircuitJob.wait_for_final_state(timeout=None)\`

## Use the websocket server to wait for the final the state of a job. The server

Expand Down Expand Up @@ -1408,7 +1408,7 @@ test("test dt tag without id", async () => {
).toMatchInlineSnapshot(`
"In addition to the public abstract methods, subclasses should also implement the following private methods:

\`classmethod _default_options()\` [GitHub](https://github.com/Qiskit/qiskit-ibm-runtime/tree/0.9.2/qiskit/providers/basicaer/qasm_simulator.py "view source code")
\`classmethod _default_options()\`

Return the default options

Expand Down
30 changes: 25 additions & 5 deletions scripts/lib/api/processHtml.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ describe("prepareGitHubLink()", () => {
test("no link", () => {
const html = `<span class="pre">None</span><span class="sig-paren">)</span><a class="headerlink" href="#qiskit_ibm_runtime.IBMBackend" title="Link to this definition">#</a>`;
const doc = Doc.load(html);
const result = prepareGitHubLink(doc.$, doc.$main);
const result = prepareGitHubLink(doc.$main, false);
expect(result).toEqual("");
doc.expectHtml(html);
});
Expand All @@ -419,9 +419,9 @@ describe("prepareGitHubLink()", () => {
const doc = Doc.load(
`<span class="pre">None</span><span class="sig-paren">)</span><a class="reference internal" href="https://ibm.com/my_link"><span class="viewcode-link"><span class="pre">[source]</span></span></a><a class="headerlink" href="#qiskit_ibm_runtime.IBMBackend" title="Link to this definition">#</a>`,
);
const result = prepareGitHubLink(doc.$, doc.$main);
const result = prepareGitHubLink(doc.$main, false);
expect(result).toEqual(
`<a href="https://ibm.com/my_link" title="view source code">GitHub</a>`,
` <a href="https://ibm.com/my_link" title="view source code">GitHub</a>`,
);
doc.expectHtml(
`<span class="pre">None</span><span class="sig-paren">)</span><a class="headerlink" href="#qiskit_ibm_runtime.IBMBackend" title="Link to this definition">#</a>`,
Expand All @@ -432,14 +432,34 @@ describe("prepareGitHubLink()", () => {
const doc = Doc.load(
`<span class="pre">None</span><span class="sig-paren">)</span><a class="reference external" href="https://github.com/Qiskit/qiskit/blob/stable/1.0/qiskit/utils/deprecation.py#L24-L101"><span class="viewcode-link"><span class="pre">[source]</span></span></a><a class="headerlink" href="#qiskit_ibm_runtime.IBMBackend" title="Link to this definition">#</a>`,
);
const result = prepareGitHubLink(doc.$, doc.$main);
const result = prepareGitHubLink(doc.$main, false);
expect(result).toEqual(
`<a href="https://github.com/Qiskit/qiskit/blob/stable/1.0/qiskit/utils/deprecation.py#L24-L101" title="view source code">GitHub</a>`,
` <a href="https://github.com/Qiskit/qiskit/blob/stable/1.0/qiskit/utils/deprecation.py#L24-L101" title="view source code">GitHub</a>`,
Copy link
Member

Choose a reason for hiding this comment

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

Is the extra space important? If so maybe a quick comment so we don't delete it at some point in the future

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is #915. I moved where the space is added as a refactor of #915.

);
doc.expectHtml(
`<span class="pre">None</span><span class="sig-paren">)</span><a class="headerlink" href="#qiskit_ibm_runtime.IBMBackend" title="Link to this definition">#</a>`,
);
});

test("method link only used when it has line numbers", () => {
const withLinesDoc = Doc.load(
`<span class="sig-paren">)</span><a class="reference external" href="https://github.com/Qiskit/qiskit-ibm-provider/tree/stable/0.10/qiskit_ibm_provider/transpiler/passes/scheduling/block_base_padder.py#L91-L117"><span class="viewcode-link"><span class="pre">[source]</span></span></a><a class="headerlink" href="#qiskit_ibm_provider.transpiler.passes.scheduling.PadDelay.run" title="Link to this definition">¶</a>`,
);
const withoutLinesDoc = Doc.load(
`<span class="sig-paren">)</span><a class="reference external" href="https://github.com/Qiskit/qiskit-ibm-provider/tree/stable/0.10/qiskit_ibm_provider/transpiler/passes/scheduling/block_base_padder.py"><span class="viewcode-link"><span class="pre">[source]</span></span></a><a class="headerlink" href="#qiskit_ibm_provider.transpiler.passes.scheduling.PadDelay.run" title="Link to this definition">¶</a>`,
);
const withLinesResult = prepareGitHubLink(withLinesDoc.$main, true);
const withoutLinesResult = prepareGitHubLink(withoutLinesDoc.$main, true);

expect(withLinesResult).toEqual(
` <a href="https://github.com/Qiskit/qiskit-ibm-provider/tree/stable/0.10/qiskit_ibm_provider/transpiler/passes/scheduling/block_base_padder.py#L91-L117" title="view source code">GitHub</a>`,
);
expect(withoutLinesResult).toEqual("");

const strippedHtml = `<span class="sig-paren">)</span><a class="headerlink" href="#qiskit_ibm_provider.transpiler.passes.scheduling.PadDelay.run" title="Link to this definition">¶</a>`;
withLinesDoc.expectHtml(strippedHtml);
withoutLinesDoc.expectHtml(strippedHtml);
});
});

describe("processMembersAndSetMeta()", () => {
Expand Down
20 changes: 15 additions & 5 deletions scripts/lib/api/processHtml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -285,10 +285,10 @@ export function processMembersAndSetMeta(
.map((child) => {
const $child = $(child);
const id = $dl.find("dt").attr("id") || "";
const github = ` ${prepareGitHubLink($, $child)}`;

const apiType = getApiType($dl);

const github = prepareGitHubLink($child, apiType === "method");

if (child.name !== "dt" || !apiType) {
return `<div>${$child.html()}</div>`;
}
Expand Down Expand Up @@ -410,15 +410,25 @@ export function processMembersAndSetMeta(
*
* This function works the same regardless of whether the Sphinx build used `sphinx.ext.viewcode`
* or `sphinx.ext.linkcode` because they have the same HTML structure.
*
* If the link corresponds to a method, we only return a link if it has line numbers included,
* which implies that the link came from `sphinx.ext.linkcode` rather than `sphinx.ext.viewcode`.
* That's because the owning class will already have a link to the relevant file; it's
* noisy and not helpful to repeat the same link without line numbers for the individual methods.
*/
export function prepareGitHubLink($: CheerioAPI, $child: Cheerio<any>): string {
export function prepareGitHubLink(
$child: Cheerio<any>,
isMethod: boolean,
): string {
const originalLink = $child.find(".viewcode-link").closest("a");
if (originalLink.length === 0) {
return "";
}
const href = originalLink.attr("href")!;
originalLink.remove();
return `<a href="${href}" title="view source code">GitHub</a>`;
originalLink.first().remove();
return !isMethod || href.includes(".py#")
? ` <a href="${href}" title="view source code">GitHub</a>`
: "";
}

export function maybeSetModuleMetadata(
Expand Down