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

Include common files like autosummary templates and gallery.css in the theme itself #262

Closed
Eric-Arellano opened this issue Apr 11, 2023 · 3 comments
Assignees
Milestone

Comments

@Eric-Arellano
Copy link
Collaborator

Right now, Terra and the Metapackage have duplicate templates for autosummary. Should we bundle those in the theme instead?

Theme users could still override our default.

Probably related to this is whether we want to stop having a dedicated page for each method. If we bundled the templates in this theme, how smooth would the migration be?

@Eric-Arellano Eric-Arellano self-assigned this Apr 11, 2023
@Eric-Arellano Eric-Arellano added this to the Theme v1.12.0 milestone Apr 20, 2023
@Eric-Arellano Eric-Arellano changed the title Figure out if we should standardize autosummary templates Include common files like autosummary templates and gallery.css in the theme itself Apr 21, 2023
Eric-Arellano added a commit that referenced this issue May 9, 2023
…302)

Part of #262.
Before, to add translation support, you would have to copy and paste
`versionutils.py`. Now, you ~only need to define `translations_list` in
`conf.py` and `qiskit_sphinx_theme` will set up the rest for you.

That duplication of `versionutils.py` was annoying to copy and also made
it too easy to fall out of sync, resulting in
#272.

To prove this works, this PR removes `language_utils.py` from
`example_docs/` and instead sets its values in `conf.py`. Translations
still work:

<img width="753" alt="Screenshot 2023-05-05 at 1 13 59 PM"
src="https://user-images.githubusercontent.com/14852634/236549631-2f844db4-8ff6-49e7-9ef0-af1cbbcb5c43.png">

This PR intentionally does not refactor any of the `translations.py`
code. It only copies and pastes what we had from all our projects, to
make it easier to review. The only divergences are:

* Removing the `translations` config option and HTML context. It wasn't
being used by anything.
* Removing the `current_translation` HTML context. It wasn't being used
by anything.
@Eric-Arellano
Copy link
Collaborator Author

Eric-Arellano commented May 11, 2023

I had ChatGPT write a script to group which projects have the same files. First, I cloned all the Qiskit projects locally, then ran fd class.rst > files.txt to get the list of files. Then, ChatGPT wrote this script:

group_files.py

import os
import hashlib


# Function to calculate MD5 Hash
def hash_file(filepath):
    hasher = hashlib.md5()
    with open(filepath, 'rb') as f:
        buf = f.read()
        hasher.update(buf)
    return hasher.hexdigest()

# Read file paths from files.txt
with open('files.txt', 'r') as f:
    files = [line.strip() for line in f]

# Dictionary to store hash: [files]
file_groups = {}

for file in files:
    file_hash = hash_file(file)
    if file_hash in file_groups:
        file_groups[file_hash].append(file)
    else:
        file_groups[file_hash] = [file]

# Print out the groups of identical files
group_number = 1
for file_group in file_groups.values():
    print(f"Group {group_number}:")
    for file in file_group:
        print(file)
    print()
    group_number += 1

No divergences

autosummary/class_no_inherited_members.rst 🚫

Files

qiskit-aqt-provider/docs/_templates/autosummary/class_no_inherited_members.rst
qiskit-dynamics/docs/_templates/autosummary/class_no_inherited_members.rst
qiskit-metapackage/docs/_templates/autosummary/class_no_inherited_members.rst
qiskit-terra/docs/_templates/autosummary/class_no_inherited_members.rst
qiskit_sphinx_theme/example_docs/docs/_templates/autosummary/class_no_inherited_members.rst

autosummary/module.rst 🚫

Files

qiskit-aer/docs/_templates/autosummary/module.rst
qiskit-experiments/docs/_templates/autosummary/module.rst
qiskit-finance/docs/_templates/autosummary/module.rst
qiskit-ibm-experiment/docs/_templates/autosummary/module.rst
qiskit-ibm-provider/docs/_templates/autosummary/module.rst
qiskit-ibm-runtime/docs/_templates/autosummary/module.rst
qiskit-ibmq-provider/docs/_templates/autosummary/module.rst
qiskit-machine-learning/docs/_templates/autosummary/module.rst
qiskit-metal/docs/_templates/autosummary/module.rst
qiskit-nature/docs/_templates/autosummary/module.rst
qiskit-optimization/docs/_templates/autosummary/module.rst
qiskit-tutorials/_templates/autosummary/module.rst

autosummary/base.rst 🚫

Files

qiskit-aer/docs/_templates/autosummary/base.rst
qiskit-experiments/docs/_templates/autosummary/base.rst
qiskit-finance/docs/_templates/autosummary/base.rst
qiskit-ibm-experiment/docs/_templates/autosummary/base.rst
qiskit-ibm-provider/docs/_templates/autosummary/base.rst
qiskit-ibm-runtime/docs/_templates/autosummary/base.rst
qiskit-ibmq-provider/docs/_templates/autosummary/base.rst
qiskit-machine-learning/docs/_templates/autosummary/base.rst
qiskit-metal/docs/_templates/autosummary/base.rst
qiskit-nature/docs/_templates/autosummary/base.rst
qiskit-optimization/docs/_templates/autosummary/base.rst
qiskit-tutorials/_templates/autosummary/base.rst

_static/custom.css 🗑️

This has some CSS rules for .toggle, which we don't ever use. Should be deleted.

Files

mthree/docs/_static/custom.css
qiskit-aqt-provider/docs/_static/custom.css
qiskit-finance/docs/_static/custom.css
qiskit-machine-learning/docs/_static/custom.css
qiskit-metal/docs/_static/custom.css
qiskit-metapackage/docs/_static/custom.css
qiskit-nature/docs/_static/custom.css
qiskit-neko/docs/_static/custom.css
qiskit-optimization/docs/_static/custom.css
qiskit-tutorials/_static/custom.css
rustworkx/docs/source/_static/custom.css

_static/no_image.png

Will provide the file _static/images/logo.png. Repos can then migrate to this (or keep using their own).

Files

qiskit-experiments/docs/_static/no_image.png
qiskit-finance/docs/_static/no_image.png
qiskit-ibm-experiment/docs/_static/no_image.png
qiskit-ibm-provider/docs/_static/no_image.png
qiskit-ibm-runtime/docs/_static/no_image.png
qiskit-machine-learning/docs/_static/no_image.png
qiskit-metal/docs/_static/no_image.png
qiskit-metapackage/docs/_static/no_image.png
qiskit-nature/docs/_static/no_image.png
qiskit-optimization/docs/_static/no_image.png
qiskit-tutorials/_static/no_image.png
qiskit_sphinx_theme/example_docs/docs/_static/no_image.png

Divergences

autosummary/class.rst 🚫

Group

Group 1:
mthree/docs/_templates/autosummary/class.rst

Group 2:
qiskit-aer/docs/_templates/autosummary/class.rst
qiskit-ibm-experiment/docs/_templates/autosummary/class.rst
qiskit-ibm-provider/docs/_templates/autosummary/class.rst
qiskit-ibm-runtime/docs/_templates/autosummary/class.rst
qiskit-ibmq-provider/docs/_templates/autosummary/class.rst
qiskit-metal/docs/_templates/autosummary/class.rst
qiskit-tutorials/_templates/autosummary/class.rst

Group 3:
qiskit-aqt-provider/docs/_templates/autosummary/class.rst
qiskit-dynamics/docs/_templates/autosummary/class.rst
qiskit-metapackage/docs/_templates/autosummary/class.rst
qiskit-terra/docs/_templates/autosummary/class.rst
qiskit_sphinx_theme/example_docs/docs/_templates/autosummary/class.rst

Group 4:
qiskit-experiments/docs/_templates/autosummary/class.rst

Group 5:
qiskit-finance/docs/_templates/autosummary/class.rst
qiskit-machine-learning/docs/_templates/autosummary/class.rst
qiskit-optimization/docs/_templates/autosummary/class.rst

Group 6:
qiskit-nature/docs/_templates/autosummary/class.rst

Group 7:
qiskit-neko/docs/_templates/autosummary/class.rst
rustworkx/docs/source/_templates/autosummary/class.rst

_static/gallery.css 🗑️

No one is even using sphinx-gallery.

Groups

Group 1: (has an orange hover color, rather than purple)
mthree/docs/_static/gallery.css

Group 2:
qiskit-experiments/docs/_static/gallery.css
qiskit-finance/docs/_static/gallery.css
qiskit-ibm-experiment/docs/_static/gallery.css
qiskit-ibm-provider/docs/_static/gallery.css
qiskit-ibm-runtime/docs/_static/gallery.css
qiskit-machine-learning/docs/_static/gallery.css
qiskit-metal/docs/_static/gallery.css
qiskit-metapackage/docs/_static/gallery.css
qiskit-nature/docs/_static/gallery.css
qiskit-neko/docs/_static/gallery.css
qiskit-optimization/docs/_static/gallery.css
qiskit-tutorials/_static/gallery.css
qiskit_sphinx_theme/example_docs/docs/_static/gallery.css

_static/style.css 🗑️

Three ReadTheDocs overrides that aren't relevant anymore.

Groups

Group 1: Almost the same as group 2, but a much lighter gray for .wy-side-scroll side scroll
mthree/docs/_static/style.css

Group 2:
qiskit-finance/docs/_static/style.css
qiskit-ibmq-provider/docs/_static/style.css
qiskit-machine-learning/docs/_static/style.css
qiskit-nature/docs/_static/style.css
qiskit-neko/docs/_static/style.css
qiskit-optimization/docs/_static/style.css
qiskit-tutorials/_static/style.css
rustworkx/docs/source/_static/style.css

Group 3: Same as group 2, only different indentation
qiskit-ibm-experiment/docs/_static/style.css
qiskit-ibm-provider/docs/_static/style.css
qiskit-ibm-runtime/docs/_static/style.css
rustworkx/docs/source/_static/gallery.css

images/logo.png ✅

Will provide this file now. Repos can still override it.

Groups

Group 1: (qiskit logo, but weird coloring)
qiskit-aer/docs/images/logo.png
qiskit-aqt-provider/docs/images/logo.png
qiskit-finance/docs/images/logo.png
qiskit-ibm-experiment/docs/images/logo.png
qiskit-ibm-provider/docs/images/logo.png
qiskit-ibm-runtime/docs/images/logo.png
qiskit-ibmq-provider/docs/images/logo.png
qiskit-machine-learning/docs/images/logo.png
qiskit-nature/docs/images/logo.png
qiskit-optimization/docs/images/logo.png
qiskit-tutorials/images/logo.png

Group 2: (metal specific)
qiskit-metal/docs/images/logo.png

Group 3: (empty)
qiskit-metapackage/docs/images/logo.png

@Eric-Arellano
Copy link
Collaborator Author

Eric-Arellano commented May 11, 2023

I couldn't get de-duplication of autosummary templates to work in #317. But I also realize I'm not certain how desirable it is. For example, I personally do not like how we do API references and would like to explore changes in the future. That is harder for us to explore if we have standardized everything in qiskit_sphinx_theme. That is, finalize the design before standardization.

--

#318 will allow us to DRY no_image.png and logo.png.

--

We already are DRYing versionutils.py via #302 and possibly #307 in the future.

--

So, all that's left I believe is removing these three CSS files that I have discovered are stale and should be removed: gallery.css, custom.css, and style.css

I'm going through all projects today to clean this up:

  • mthree
  • qiskit-aer
  • qiskit-aqt-provider
  • qiskit-dynamics
  • qiskit-experiments
  • qiskit-finance
  • qiskit-ibm-experiment
  • qiskit-ibm-provider
  • qiskit-ibm-runtime
  • qiskit-ibmq-provider
  • qiskit-ionq
  • qiskit-machine-learning
  • qiskit-metal
  • qiskit-metapackage
  • qiskit-nature
  • qiskit-neko
  • qiskit-optimization
  • qiskit-terra
  • qiskit-tutorials
  • rustworkx

Eric-Arellano added a commit that referenced this issue May 11, 2023
…opying `no_image.png` (#318)

Part of #262.

14 projects are copying around the same `no_image.png` file. It's
helpful so that you can set `nbsphinx_thumbnails` to point to it. This
PR adds the file to `qiskit_sphinx_theme` so that they can instead use
the standard file if they want.

They don't have to switch to this file. And repos can also still
override `images/logo.png` how they want, e.g. that Metal has it be a
Metal specific logo currently. Their override will take precedence.

--

Note that we already distribute `static/images/logo.svg`. But
`nbsphinx_thumbnails` doesn't work with an SVG.

--

This PR also removes `gallery.css` from the example docs. We don't use
`sphinx-gallery`, so this was a bad copy and paste.
@Eric-Arellano
Copy link
Collaborator Author

Every repo has now been updated to remove gallery.css, custom.css, and style.css.

With Sphinx theme 1.12, I will change them to use the provided logo.png.

I could not de-duplicate favicon.ico in #324.

The only other de-duplication is custom_directives.py (#323) and logic for Previous Releases #307.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant