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.

Sign up for GitHub

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

Jump to bottom

Improve header hierarchy for API module pages #12325

Merged
merged 4 commits into from
May 6, 2024
Merged

Improve header hierarchy for API module pages #12325

merged 4 commits into from
May 6, 2024

Conversation

Eric-Arellano
Copy link
Collaborator

@Eric-Arellano Eric-Arellano commented May 1, 2024 •
edited
Loading

Closes Qiskit/documentation#942. This uses a few principles:

  • There should never be more than one h1, as it violates web best practices.
  • Functions, classes, and exceptions should always be preceded by at least an h2
    • Why? The docs app generates a header for functions/classes/exceptions, which is important for a) anchor links, b) search, and c) the right page table of contents. But the header will always be an h3 or lower (like h4). It's a semantic violation to go from h1 to h3 - you are supposed to have an intermediate header
  • Flatten headers when possible
    • The docs app only shows h2 and h3 in the right page ToC, and it will always be that way because showing h4s makes things too noisy when we experimented. There are a few instances where content is meaningful enough it should show up in the page ToC
    • There were a few instances of multiple headers with only one entry each. This is noisy.
  • Use standard RST convention for headers
    • h1 uses = above and below
    • h2 uses = below
    • h3 uses - below
    • h4 uses ~ below

@Eric-Arellano Eric-Arellano requested review from eggerdj, wshanks, a team and jyu00 as code owners May 1, 2024 21:01
@qiskit-bot
Copy link
Collaborator

One or more of the the following people are requested to review this:

  • @Cryoris
  • @Qiskit/terra-core
  • @ajavadia
  • @mtreinish
  • @nkanazawa1989

@Eric-Arellano Eric-Arellano added documentation Something is not clear or an error documentation stable backport potential The bug might be minimal and/or import enough to be port to stable Changelog: None Do not include in changelog labels May 1, 2024
@Eric-Arellano Eric-Arellano added this to the 1.1.0 milestone May 1, 2024
Eric-Arellano
Eric-Arellano commented May 1, 2024
View reviewed changes
qiskit/assembler/__init__.py
Comment on lines -20 to -21
Circuit Assembler
=================
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These headers each only had one entry. It makes the page a bit noisy: https://docs.quantum.ibm.com/api/qiskit/dev/assembler

qiskit/circuit/__init__.py
Comment on lines -206 to -207
API overview of :mod:`qiskit.circuit`
=====================================
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This breaks the docs app header:

Screenshot 2024-05-01 at 5 03 56 PM

qiskit/circuit/classical/types/__init__.py
Comment on lines -56 to +57
-------------------------
=========================
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is to reduce the amount of nesting

qiskit/circuit/classicalfunction/__init__.py
@@ -81,6 +81,7 @@ def grover_oracle(a: Int1, b: Int1, c: Int1, d: Int1) -> Int1:

Decorator for a classical function that returns a `ClassicalFunction` object.

.. autofunction:: classical_function
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was unintentionally left off before

qiskit/circuit/library/__init__.py
.. autosummary::

Barrier
* :class:`qiskit.circuit.Barrier`
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We now link to the inline class definition

qiskit/providers/basic_provider/__init__.py Outdated Show resolved Hide resolved
qiskit/providers/models/__init__.py
@@ -19,8 +19,8 @@

Qiskit schema-conformant objects used by the backends and providers.

Backend Objects
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Renamed to not repeat "Backend Objects", which is already the h1

qiskit/qpy/__init__.py
@@ -11,9 +11,9 @@
# that they have been altered from the originals.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The changes in this file are to use standard conventions for headers

qiskit/scheduler/__init__.py
.. autoclass:: ScheduleConfig

.. currentmodule:: qiskit.scheduler.schedule_circuit
.. autofunction:: schedule_circuit
.. currentmodule:: qiskit.scheduler

.. automodule:: qiskit.scheduler.methods
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 inlined the module docstring and added a new header. Before, it was paragraph text and looked weird.

qiskit/transpiler/passes/__init__.py
@@ -152,8 +152,10 @@
HLSConfig
SolovayKitaev

Post Layout (Post transpile qubit selection)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This made the page ToC quite long

@coveralls
Copy link

coveralls commented May 1, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 8968536423

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • 7 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.03%) to 89.646%
Files with Coverage Reduction New Missed Lines %
qiskit/transpiler/passes/synthesis/unitary_synthesis.py 3 88.02%
crates/qasm2/src/lex.rs 4 93.13%
Totals Coverage Status
Change from base Build 8966541600: 0.03%
Covered Lines: 62258
Relevant Lines: 69449
💛 - Coveralls

@mtreinish mtreinish added the on hold Can not fix yet label May 2, 2024
@Eric-Arellano Eric-Arellano mentioned this pull request May 2, 2024
Merged
@mtreinish mtreinish removed the on hold Can not fix yet label May 3, 2024
ElePT
ElePT previously approved these changes May 6, 2024
View reviewed changes
Copy link
Contributor

@ElePT ElePT left a comment

Choose a reason for hiding this comment

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

LGTM.

qiskit/converters/__init__.py Outdated Show resolved Hide resolved
@Eric-Arellano Eric-Arellano requested a review from ElePT May 6, 2024 11:50
@ElePT ElePT enabled auto-merge May 6, 2024 12:11
@ElePT ElePT added this pull request to the merge queue May 6, 2024
Merged via the queue into Qiskit:main with commit f39c907 May 6, 2024
15 checks passed
mergify bot pushed a commit that referenced this pull request May 6, 2024
@Eric-Arellano @mergify
Improve header hierarchy for API module pages (#12325)
f86a69a 
* Improve header hierarchy for API module pages

* Tweaks

* Review feedback

(cherry picked from commit f39c907)
@mergify mergify bot mentioned this pull request May 6, 2024
Merged
@Eric-Arellano Eric-Arellano deleted the EA/module-hierarchy branch May 6, 2024 15:05
@Eric-Arellano Eric-Arellano mentioned this pull request May 6, 2024
Closed
33 tasks
github-merge-queue bot pushed a commit that referenced this pull request May 9, 2024
@mergify @Eric-Arellano
Improve header hierarchy for API module pages (#12325) (#12350)
2cde2d6 
* Improve header hierarchy for API module pages

* Tweaks

* Review feedback

(cherry picked from commit f39c907)

Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
ElePT pushed a commit to ElePT/qiskit that referenced this pull request May 31, 2024
@Eric-Arellano @ElePT
Improve header hierarchy for API module pages (Qiskit#12325)
a840e0d 
* Improve header hierarchy for API module pages

* Tweaks

* Review feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ElePT ElePT ElePT approved these changes

@eggerdj eggerdj Awaiting requested review from eggerdj eggerdj is a code owner

@wshanks wshanks Awaiting requested review from wshanks wshanks is a code owner

@jyu00 jyu00 Awaiting requested review from jyu00 jyu00 is a code owner

Assignees
No one assigned
Labels
Changelog: None Do not include in changelog documentation Something is not clear or an error documentation stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
None yet
Milestone
1.1.0
Development

Successfully merging this pull request may close these issues.

Initial audit of header hierarchy for all module API pages
5 participants
@Eric-Arellano @qiskit-bot @coveralls @ElePT @mtreinish