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 objects.inv #522

Merged
merged 51 commits into from
Jan 17, 2024
Merged

Add objects.inv #522

merged 51 commits into from
Jan 17, 2024

Conversation

frankharkins
Copy link
Member

@frankharkins frankharkins commented Dec 14, 2023

  • Added objectsInv.ts to read, decompress, and parse objects.inv files (and vice versa)
  • Updated updateApiDocs.ts to copy the file over from the artifact
  • Added logic to transform links and delete any references that don't exist in our docs

Explanation

The objects.inv file is an index that intersphinx uses to create cross-references to our documentation. To allow other projects to cross-reference Qiskit, we need to host this file.

For a description of the file's data structure, see the sphobjinv documentation, whose implementation I copied when working out how to transform this file.

@frankharkins
Copy link
Member Author

frankharkins commented Jan 10, 2024

Update

The following links are broken according to the link checker:

  • qiskit-ibm-provider:

    These links do seem to exist, so I believe this is a bug in the link checker related to all-caps anchors.

    api/qiskit-ibm-provider/qiskit_ibm_provider.transpiler.passes.scheduling.DynamicCircuitInstructionDurations#MEASURE_PATCH_CYCLES
    api/qiskit-ibm-provider/qiskit_ibm_provider.transpiler.passes.scheduling.DynamicCircuitInstructionDurations#MEASURE_PATCH_ODD_OFFSET
    
  • qiskit-ibm-runtime:

    The first should be fixed with API generation regression in properties #224. The others point to index when they should point to runtime_service. I'm still working out the appropriate place to fix this.

    /api/qiskit-ibm-runtime/qiskit_ibm_runtime.RuntimeEncoder#key_separator
    /api/qiskit-ibm-runtime/index#next-steps
    /api/qiskit-ibm-runtime/index#qiskit-runtime-version-api-docs-preview
    
  • qiskit

    These seem to be a mixture of the all-caps (potential) bug I mentioned previously, plus problems mapping the old release notes to the new ones, plus some other bugs I still need to identify.

    EDIT: Also some pages that exist in the old docs that we don't host (e.g. /api/qiskit/qc_intro, /api/qiskit/faq); removing these reduces the broken links from 1642 -> 1554)

    EDIT: Removing release note references takes this down to 55 broken links

    (55 items)
    /api/qiskit/circuit#qiskit.circuit.CASE_DEFAULT
    /api/qiskit/qiskit.passmanager.FlowController#registered_controllers
    /api/qiskit/qiskit.providers.basicaer.QasmSimulatorPy#default_configuration
    /api/qiskit/qiskit.providers.basicaer.QasmSimulatorPy#default_options
    /api/qiskit/qiskit.providers.basicaer.StatevectorSimulatorPy#default_configuration
    /api/qiskit/qiskit.providers.basicaer.StatevectorSimulatorPy#default_options
    /api/qiskit/qiskit.providers.basicaer.UnitarySimulatorPy#default_configuration
    /api/qiskit/qiskit.providers.basicaer.UnitarySimulatorPy#default_options
    /api/qiskit/qiskit.pulse.instructions.Reference#scope_delimiter
    /api/qiskit/qasm2#qiskit.qasm2.LEGACY_CUSTOM_CLASSICAL
    /api/qiskit/qasm2#qiskit.qasm2.LEGACY_CUSTOM_INSTRUCTIONS
    /api/qiskit/qasm2#qiskit.qasm2.LEGACY_INCLUDE_PATH
    /api/qiskit/qiskit.transpiler.StagedPassManager#invalid_stage_regex
    /api/qiskit/utils#qiskit.utils.algorithm_globals
    /api/qiskit/utils#qiskit.utils.optionals.HAS_AER
    /api/qiskit/utils#qiskit.utils.optionals.HAS_CONSTRAINT
    /api/qiskit/utils#qiskit.utils.optionals.HAS_CPLEX
    /api/qiskit/utils#qiskit.utils.optionals.HAS_CVXPY
    /api/qiskit/utils#qiskit.utils.optionals.HAS_DOCPLEX
    /api/qiskit/utils#qiskit.utils.optionals.HAS_FIXTURES
    /api/qiskit/utils#qiskit.utils.optionals.HAS_GRAPHVIZ
    /api/qiskit/utils#qiskit.utils.optionals.HAS_IBMQ
    /api/qiskit/utils#qiskit.utils.optionals.HAS_IGNIS
    /api/qiskit/utils#qiskit.utils.optionals.HAS_IPYTHON
    /api/qiskit/utils#qiskit.utils.optionals.HAS_IPYWIDGETS
    /api/qiskit/utils#qiskit.utils.optionals.HAS_JAX
    /api/qiskit/utils#qiskit.utils.optionals.HAS_JUPYTER
    /api/qiskit/utils#qiskit.utils.optionals.HAS_MATPLOTLIB
    /api/qiskit/utils#qiskit.utils.optionals.HAS_NETWORKX
    /api/qiskit/utils#qiskit.utils.optionals.HAS_NLOPT
    /api/qiskit/utils#qiskit.utils.optionals.HAS_PDFLATEX
    /api/qiskit/utils#qiskit.utils.optionals.HAS_PDFTOCAIRO
    /api/qiskit/utils#qiskit.utils.optionals.HAS_PIL
    /api/qiskit/utils#qiskit.utils.optionals.HAS_PYDOT
    /api/qiskit/utils#qiskit.utils.optionals.HAS_PYGMENTS
    /api/qiskit/utils#qiskit.utils.optionals.HAS_PYLATEX
    /api/qiskit/utils#qiskit.utils.optionals.HAS_QASM3_IMPORT
    /api/qiskit/utils#qiskit.utils.optionals.HAS_SEABORN
    /api/qiskit/utils#qiskit.utils.optionals.HAS_SKLEARN
    /api/qiskit/utils#qiskit.utils.optionals.HAS_SKQUANT
    /api/qiskit/utils#qiskit.utils.optionals.HAS_SQSNOBFIT
    /api/qiskit/utils#qiskit.utils.optionals.HAS_SYMENGINE
    /api/qiskit/utils#qiskit.utils.optionals.HAS_TESTTOOLS
    /api/qiskit/utils#qiskit.utils.optionals.HAS_TOQM
    /api/qiskit/utils#qiskit.utils.optionals.HAS_TWEEDLEDUM
    /api/qiskit/utils#qiskit.utils.optionals.HAS_Z3
    /api/qiskit/index#main-qiskit-related-projects
    /api/qiskit/index#qiskit-version-documentation
    /api/qiskit/qiskit.visualization.timeline_drawer#style-dict-doc
    /api/qiskit/qiskit.pulse.library.SymbolicPulse#symbolic-pulse-constraints
    /api/qiskit/qiskit.pulse.library.SymbolicPulse#symbolic-pulse-envelope
    /api/qiskit/qiskit.pulse.library.SymbolicPulse#symbolic-pulse-eval-condition
    /api/qiskit/qiskit.pulse.library.SymbolicPulse#symbolic-pulse-serialize
    /api/qiskit-ibm-runtime/qiskit_ibm_runtime.RuntimeEncoder#key_separator
    /api/qiskit-ibm-runtime/index#next-steps
    /api/qiskit-ibm-runtime/index#qiskit-runtime-version-api-docs-preview
    

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the number of broken links is low enough I can ignore them here rather than disabling checking completely. The advantages are:

  • We're less likely to forget about objects.inv when we decide to address the broken links
  • We'll still be alerted to new broken links in future objects.inv files

Copy link
Collaborator

@arnaucasau arnaucasau left a comment

Choose a reason for hiding this comment

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

Awesome work, thank you Frank! Just one question, I was testing it regenerating a historical version and the script generated the file public/api/qiskit/0.44 instead of public/api/qiskit/0.44/objects.inv. Is that the intended behavior?

I think the reason is the extname thinks .44 is the extension of a file and it doesn't join the path with objects.inv in objectsInv.ts.

async write(path: string): Promise<void> {
    if (extname(path) === "") {
      path = join(path, "objects.inv");
    }

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.

Looking good! Thanks for enabling the link checker - good call. I think this is ready to merge after addressing Arnau's concern.

scripts/commands/updateApiDocs.ts Show resolved Hide resolved
scripts/lib/api/objectsInv.ts Show resolved Hide resolved
scripts/lib/api/objectsInv.ts Outdated Show resolved Hide resolved
scripts/lib/api/objectsInv.ts Outdated Show resolved Hide resolved
scripts/lib/api/objectsInv.ts Outdated Show resolved Hide resolved
scripts/lib/api/objectsInv.ts Outdated Show resolved Hide resolved
scripts/lib/api/updateLinks.test.ts Show resolved Hide resolved
frankharkins and others added 5 commits January 17, 2024 17:23
Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
@frankharkins
Copy link
Member Author

Thanks both for the reviews!

@arnaucasau good catch with the historical apis script; it's working now.

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.

Amazing job 🙌

scripts/lib/api/objectsInv.ts Outdated Show resolved Hide resolved
Comment on lines +90 to +92
if (filePath.endsWith(".inv")) {
return await parseObjectsInv(filePath);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perfect 🙌

@frankharkins frankharkins added this pull request to the merge queue Jan 17, 2024
Merged via the queue into main with commit e09cfcf Jan 17, 2024
2 checks passed
@frankharkins frankharkins deleted the FH/objects.inv branch January 17, 2024 19:01
github-merge-queue bot pushed a commit that referenced this pull request Jan 18, 2024
The API generation script is now able to work with `objects.inv` files
thanks to @frankharkins at
#522. This PR allows us to
copy over the objects.file when we want to make a version historical
running `npm run make-historical -- -p <projectName>`.
frankharkins added a commit to frankharkins/documentation that referenced this pull request Jul 22, 2024
This is prework for Qiskit#522,
which adds `sphinx.inv`. We want to check the links contained in
`sphinx.inv`, but we don't expect anyone to have links pointing to
`sphinx.inv`.

Our original abstractions made it hard to model `objects.inv` correctly
with the link checker. Now, `markdown.ts` is renamed to
`extractLinks.ts` and it solely deals with parsing files. It no longer
has a bad coupling to the complex `linksToOriginFiles` variable from
`FileBatch.ts`.

This should make it much more obvious how to handle `objects.inv`.

---------

Co-authored-by: Frank Harkins <frankharkins@hotmail.co.uk>
frankharkins added a commit to frankharkins/documentation that referenced this pull request Jul 22, 2024
- Added `objectsInv.ts` to read, decompress, and parse `objects.inv`
files (and vice versa)
- Updated `updateApiDocs.ts` to copy the file over from the artifact
- Added logic to transform links and delete any references that don't
exist in our docs

### Explanation

The `objects.inv` file is an index that
[intersphinx](https://www.sphinx-doc.org/en/master/usage/extensions/intersphinx.html)
uses to create cross-references to our documentation. To allow other
projects to cross-reference Qiskit, we need to host this file.

For a description of the file's data structure, see the [sphobjinv
documentation](https://sphobjinv.readthedocs.io/en/stable/syntax.html),
whose implementation I copied when working out how to transform this
file.

---------

Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
frankharkins pushed a commit to frankharkins/documentation that referenced this pull request Jul 22, 2024
The API generation script is now able to work with `objects.inv` files
thanks to @frankharkins at
Qiskit#522. This PR allows us to
copy over the objects.file when we want to make a version historical
running `npm run make-historical -- -p <projectName>`.
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.

3 participants