Skip to content

Commit

Permalink
Don't truncate API ToC by default (#1318)
Browse files Browse the repository at this point in the history 
Turns out, this truncation does not work well with
#1317. We agreed the best
approach is to show the full module path for that package.

This PR also fixes #1308 to
put the error in the correct location.
  • Loading branch information
@Eric-Arellano
Eric-Arellano authored May 8, 2024
1 parent 87e7cac commit e2e765a
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 56 deletions.
36 changes: 24 additions & 12 deletions scripts/lib/api/Pkg.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@ export class ReleaseNotesConfig {
}
}

export class TocConfig {
readonly truncate: boolean;
readonly tocGrouping?: TocGrouping;

constructor(kwargs: { truncate?: boolean; tocGrouping?: TocGrouping }) {
this.truncate = kwargs.truncate ?? false;
this.tocGrouping = kwargs.tocGrouping;
}
}

type PackageType = "latest" | "historical" | "dev";

/**
Expand All @@ -40,7 +50,7 @@ export class Pkg {
readonly versionWithoutPatch: string;
readonly type: PackageType;
readonly releaseNotesConfig: ReleaseNotesConfig;
readonly tocGrouping?: TocGrouping;
readonly tocConfig: TocConfig;

static VALID_NAMES = ["qiskit", "qiskit-ibm-runtime", "qiskit-ibm-provider"];

Expand All @@ -52,7 +62,7 @@ export class Pkg {
versionWithoutPatch: string;
type: PackageType;
releaseNotesConfig?: ReleaseNotesConfig;
tocGrouping?: TocGrouping;
tocConfig?: TocConfig;
}) {
this.name = kwargs.name;
this.title = kwargs.title;
Expand All @@ -62,7 +72,7 @@ export class Pkg {
this.type = kwargs.type;
this.releaseNotesConfig =
kwargs.releaseNotesConfig ?? new ReleaseNotesConfig({});
this.tocGrouping = kwargs.tocGrouping;
this.tocConfig = kwargs.tocConfig ?? new TocConfig({});
}

static async fromArgs(
Expand All @@ -88,7 +98,7 @@ export class Pkg {
releaseNotesConfig: new ReleaseNotesConfig({
separatePagesVersions: releaseNoteEntries,
}),
tocGrouping: QISKIT_TOC_GROUPING,
tocConfig: new TocConfig({ tocGrouping: QISKIT_TOC_GROUPING }),
});
}

Expand All @@ -98,6 +108,7 @@ export class Pkg {
title: "Qiskit Runtime IBM Client",
name: "qiskit-ibm-runtime",
githubSlug: "qiskit/qiskit-ibm-runtime",
tocConfig: new TocConfig({ truncate: true }),
});
}

Expand All @@ -107,6 +118,7 @@ export class Pkg {
title: "Qiskit IBM Provider (deprecated)",
name: "qiskit-ibm-provider",
githubSlug: "qiskit/qiskit-ibm-provider",
tocConfig: new TocConfig({ truncate: true }),
});
}

Expand All @@ -121,7 +133,7 @@ export class Pkg {
versionWithoutPatch?: string;
type?: PackageType;
releaseNotesConfig?: ReleaseNotesConfig;
tocGrouping?: TocGrouping;
tocConfig?: TocConfig;
}): Pkg {
return new Pkg({
name: kwargs.name ?? "my-quantum-project",
Expand All @@ -131,7 +143,7 @@ export class Pkg {
versionWithoutPatch: kwargs.versionWithoutPatch ?? "0.1",
type: kwargs.type ?? "latest",
releaseNotesConfig: kwargs.releaseNotesConfig,
tocGrouping: kwargs.tocGrouping ?? undefined,
tocConfig: kwargs.tocConfig,
});
}

Expand Down Expand Up @@ -186,12 +198,6 @@ export class Pkg {
* `sphinx.ext.viewcode`, which means we need to deal with its quirks like handling `__init__.py`.
*/
determineGithubUrlFn(): (fileName: string) => string {
if (!this.githubSlug) {
throw new Error(
`Encountered sphinx.ext.viewcode link, but Pkg.githubSlug is not set for ${this.name}`,
);
}

// For files like `my_module/__init__.py`, `sphinx.ext.viewcode` will title the
// file `my_module.py`. We need to add back the `/__init__.py` when linking to GitHub.
const convertToInitPy = new Set([
Expand Down Expand Up @@ -219,6 +225,12 @@ export class Pkg {
: `stable/${this.versionWithoutPatch}`;
const baseUrl = `https://github.com/${this.githubSlug}/tree/${branchName}`;
return (fileName) => {
if (!this.githubSlug) {
throw new Error(
`Encountered sphinx.ext.viewcode link, but Pkg.githubSlug is not set for ${this.name}`,
);
}

return `${baseUrl}/${normalizeFile(fileName)}.py`;
};
}
Expand Down
113 changes: 79 additions & 34 deletions scripts/lib/api/generateToc.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import { describe, expect, test } from "@jest/globals";

import { generateToc } from "./generateToc";
import { Pkg, ReleaseNotesConfig } from "./Pkg";
import { Pkg, ReleaseNotesConfig, TocConfig } from "./Pkg";
import type { TocGroupingEntry } from "./TocGrouping";

const DEFAULT_ARGS = {
Expand Down Expand Up @@ -99,15 +99,15 @@ describe("generateToc", () => {
url: "/api/my-quantum-project",
},
{
title: "...options",
title: "my_quantum_project.options",
url: "/api/my-quantum-project/options",
},
{
title: "...options.submodule",
title: "my_quantum_project.options.submodule",
url: "/api/my-quantum-project/my_quantum_project.options.submodule",
},
{
title: "...single",
title: "my_quantum_project.single",
url: "/api/my-quantum-project/single",
},
{
Expand All @@ -120,6 +120,48 @@ describe("generateToc", () => {
});
});

test("truncate module names", () => {
const toc = generateToc(
Pkg.mock({
tocConfig: new TocConfig({ truncate: true }),
releaseNotesConfig: new ReleaseNotesConfig({ enabled: false }),
}),
[
{
meta: {
apiType: "module",
apiName: "my_quantum_project",
},
url: "/api/my-quantum-project",
...DEFAULT_ARGS,
},
{
meta: {
apiType: "module",
apiName: "my_quantum_project.options",
},
url: "/api/my-quantum-project/options",
...DEFAULT_ARGS,
},
],
);

expect(toc).toEqual({
children: [
{
title: "my_quantum_project",
url: "/api/my-quantum-project",
},
{
title: "...options",
url: "/api/my-quantum-project/options",
},
],
collapsed: true,
title: "My Quantum Project",
});
});

test("TOC with grouped modules", () => {
// This ordering is intentional.
const topLevelEntries: TocGroupingEntry[] = [
Expand All @@ -145,40 +187,43 @@ describe("generateToc", () => {
module == "my_quantum_project.module" ? "Group 1" : "Group 2",
};

const toc = generateToc(Pkg.mock({ tocGrouping }), [
{
meta: {
apiType: "module",
apiName: "my_quantum_project",
const toc = generateToc(
Pkg.mock({ tocConfig: new TocConfig({ tocGrouping }) }),
[
{
meta: {
apiType: "module",
apiName: "my_quantum_project",
},
url: "/api/my-quantum-project",
...DEFAULT_ARGS,
},
url: "/api/my-quantum-project",
...DEFAULT_ARGS,
},
{
meta: {
apiType: "module",
apiName: "my_quantum_project.module",
{
meta: {
apiType: "module",
apiName: "my_quantum_project.module",
},
url: "/api/my-quantum-project/my_quantum_project.module",
...DEFAULT_ARGS,
},
url: "/api/my-quantum-project/my_quantum_project.module",
...DEFAULT_ARGS,
},
{
meta: {
apiType: "module",
apiName: "my_quantum_project.module.submodule",
{
meta: {
apiType: "module",
apiName: "my_quantum_project.module.submodule",
},
url: "/api/my-quantum-project/my_quantum_project.module.submodule",
...DEFAULT_ARGS,
},
url: "/api/my-quantum-project/my_quantum_project.module.submodule",
...DEFAULT_ARGS,
},
{
meta: {
apiType: "module",
apiName: "my_quantum_project.another",
{
meta: {
apiType: "module",
apiName: "my_quantum_project.another",
},
url: "/api/my-quantum-project/my_quantum_project.another",
...DEFAULT_ARGS,
},
url: "/api/my-quantum-project/my_quantum_project.another",
...DEFAULT_ARGS,
},
]);
],
);
expect(toc).toEqual({
collapsed: true,
title: "My Quantum Project",
Expand Down
25 changes: 15 additions & 10 deletions scripts/lib/api/generateToc.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ export function generateToc(pkg: Pkg, results: HtmlToMdResultWithUrl[]): Toc {

addItemsToModules(items, tocModulesByTitle, tocModuleTitles);

const orderedEntries = pkg.tocGrouping
? groupAndSortModules(pkg.tocGrouping, tocModulesByTitle)
: sortAndTruncateModules(tocModules);
const orderedEntries = pkg.tocConfig.tocGrouping
? groupAndSortModules(pkg.tocConfig.tocGrouping, tocModulesByTitle)
: sortAndMaybeTruncateModules(tocModules, pkg.tocConfig.truncate);

generateOverviewPage(tocModules);
const maybeIndexPage = ensureIndexPage(pkg, orderedEntries);
Expand Down Expand Up @@ -179,17 +179,22 @@ function groupAndSortModules(
return result;
}

/** Sorts all modules and truncates the package name, e.g. `qiskit_ibm_runtime.options` -> `...options`.
/** Sorts all modules and possibly truncates the package name, e.g. `qiskit_ibm_runtime.options` -> `...options`.
*
* Returns a flat list of modules without any nesting.
*/
function sortAndTruncateModules(entries: TocEntry[]): TocEntry[] {
function sortAndMaybeTruncateModules(
entries: TocEntry[],
truncate: boolean,
): TocEntry[] {
const sorted = orderEntriesByTitle(entries);
sorted.forEach((entry) => {
// E.g. qiskit_ibm_runtime.options -> ...options, but ignore
// qiskit_ibm_runtime without a `.`.
entry.title = entry.title.replace(/^[^.]+\./, "...");
});
if (truncate) {
sorted.forEach((entry) => {
// E.g. qiskit_ibm_runtime.options -> ...options, but ignore
// qiskit_ibm_runtime without a `.`.
entry.title = entry.title.replace(/^[^.]+\./, "...");
});
}
return sorted;
}

Expand Down

0 comments on commit e2e765a

Please sign in to comment.