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

Allow enabling translations by setting translations_list in config #302

Merged
merged 8 commits into from
May 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
32 changes: 32 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ This package is available on PyPI using:
```bash
pip install qiskit-sphinx-theme
```

Then, set up the theme by updating `conf.py`:

1. Set `html_theme = "qiskit_sphinx_theme"`
2. Add `"qiskit_sphinx_theme"` to `extensions`

## Configure Left Sidebar

To keep UX/UI similar across different Qiskit packages we strongly encourage the following structure for you sidebar, which can be set in the toctree of your `index.rst`:
Expand Down Expand Up @@ -118,7 +124,33 @@ To configure your documentation to use exapandable sidebar headings like the exa

*Tip: if you want to add all files in a sub-directory to your expandable dropdown section you can use the `:glob:` directive instead of listing out each page (see example above for the How-to Guides, API Reference and Explanations sections). This will list out each page in alphabetical order, so if you want a specific order you will need to list the pages out individually in the `toctree` (see example above for the Tutorials section)*

## Enable translations

First, coordinate with the Translations team at https://github.com/qiskit-community/qiskit-translations to express your interest and to coordinate setting up the infrastructure.

Once the Translations team is ready, then update your `conf.py`:

* Ensure that `qiskit_sphinx_theme` is in the `extensions` setting.
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this step only necessary for enabling translations? Or should it be in extensions anyway?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sort of. If you don't add qiskit_sphinx_theme to extensions, then you need to add sphinxcontrib.jquery to the extensions list because Pytorch requires jQuery. At least until we switch to Furo.

So, I think we can simplify things to say that you should always add qiskit_sphinx_theme to the extensions list.

* Set the option `translations_list` to a list of pairs of the locale code and the language name, e.g. `[..., ("de_DE", "German")]`.
* Set the option `content_prefix` to your project's URL prefix, like `ecosystem/finance`.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are a couple of things I don't like about this option, including the name not being clear what it is: #303.

I plan to make some improvements as part of Sphinx Theme 0.12. But I want to do that in a follow up PR to make the code review easier.

For now, this simply copies the name and logic we had in all of our Terra and Ecosystem projects.

As long as we make any changes before 0.12 is released, it is safe for us to make breaking changes like renaming this.


For example:

```python
extensions = [
...,
"qiskit_sphinx_theme",
]

translations_list = [
('en', 'English'),
('bn_BN', 'Bengali'),
('fr_FR', 'French'),
('de_DE', 'German'),
]

content_prefix = "ecosystem/finance"
```

## Enable Qiskit.org Analytics

Expand Down
20 changes: 11 additions & 9 deletions example_docs/docs/conf.py
Eric-Arellano marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# This code is part of Qiskit.
#
# (C) Copyright IBM 2018.
# (C) Copyright IBM 2018, 2023.
#
# This code is licensed under the Apache License, Version 2.0. You may
# obtain a copy of this license in the LICENSE.txt file in the root directory
Expand All @@ -13,8 +13,7 @@
import os
import sys

# This allows autodoc to find the `api_example` folder and
# for us to register our `docs.language_utils` extension.
# This allows autodoc to find the `api_example` folder.
sys.path.insert(0, os.path.abspath(".."))

project = 'Qiskit sphinx theme'
Expand All @@ -41,7 +40,7 @@
'jupyter_sphinx',
'sphinx_design',
"nbsphinx",
"sphinxcontrib.jquery",
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 figured out now why jQuery wasn't being activated automatically by qiskit-sphinx-theme like I hoped for in #277. It's not enough to set html_theme = qiskit_sphinx_theme. We need to also add it to extensions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

so does this mean we don't need sphinxcontrib.jquery anymore?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If you set qiskit_sphinx_theme in extensions, then it is redundant to also include sphinxcontrib.jquery. That is because we have this code that tells our own extension to also activate jQuery:

# Sphinx 6 stopped including jQuery by default. Our Pytorch theme depend on jQuery,
# so install it for our users automatically.
app.setup_extension("sphinxcontrib.jquery")

"qiskit_sphinx_theme",
]

html_last_updated_fmt = '%Y/%m/%d'
Expand Down Expand Up @@ -71,6 +70,14 @@
'expandable_sidebar': True
}

# When creating a new repo, follow the instructions in this repo's README.md on
# `Enable translations`. Remove this value if you aren't using translations.
translations_list = [
('en', 'English'),
('bn_BN', 'Bengali'),
('fr_FR', 'French'),
]

# This allows RST files to put `|version|` in their file and
# have it updated with the release set in conf.py.
rst_prolog = f"""
Expand All @@ -96,8 +103,3 @@
nbsphinx_thumbnails = {
"sphinx_guide/notebook": "_static/no_image.png",
}


def setup(app):
"""Entry point for Sphinx extensions."""
app.setup_extension('docs.language_utils')
14 changes: 10 additions & 4 deletions qiskit_sphinx_theme/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
from pathlib import Path
from warnings import warn

from qiskit_sphinx_theme import translations

__version__ = '1.11.0rc1'
__version_full__ = __version__

Expand Down Expand Up @@ -32,15 +34,15 @@ def get_html_theme_path():

# See https://www.sphinx-doc.org/en/master/development/theming.html
def setup(app):
# Sphinx 6 stopped including jQuery by default. Our Pytorch theme depend on jQuery,
# so install it for our users automatically.
app.setup_extension("sphinxcontrib.jquery")
# We always activate translations support, but users need to set `translations_list` in config
# for it to be used.
translations.setup(app)

app.add_html_theme("qiskit_sphinx_theme", _get_theme_absolute_path("pytorch_base"))
app.add_html_theme("_qiskit_furo", _get_theme_absolute_path("furo/base"))

# The below must be kept in sync with `furo/__init__.py`.
if app.config.html_theme == "_qiskit_furo":
# The below must be kept in sync with `furo/__init__.py`.
from furo import (
WrapTableAndMathInAContainerTransform,
_builder_inited,
Expand All @@ -52,5 +54,9 @@ def setup(app):
app.connect("html-page-context", _html_page_context)
app.connect("builder-inited", _builder_inited)
app.connect("build-finished", _overwrite_pygments_css)
else:
# Sphinx 6 stopped including jQuery by default. Our Pytorch theme depend on jQuery,
# so activate it for our users automatically.
app.setup_extension("sphinxcontrib.jquery")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No need to activate jQuery for Furo.


return {'parallel_read_safe': True, 'parallel_write_safe': True}
Eric-Arellano marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

have you cross checked this file with https://github.com/Qiskit/qiskit-metapackage/blob/master/docs/versionutils.py? I think there are a couple of functions used in qiskit that aren't here, e.g. _get_language_label and _get_translation_url. I'm not really sure what those functions do but it might be important if it's used in qiskit

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Those are here too :) This is based on that file, and only remove the Git/Previous Versions code. Along with what I mention in the PR description about translations config value and current_translation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh! I didn't expand the file view in the github diff 😅 my bad, I see them now

Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

# This code is part of Qiskit.
#
# (C) Copyright IBM 2018.
# (C) Copyright IBM 2018, 2023.
#
# This code is licensed under the Apache License, Version 2.0. You may
# obtain a copy of this license in the LICENSE.txt file in the root directory
Expand All @@ -12,50 +12,28 @@
# copyright notice, and modified files need to carry a notice indicating
# that they have been altered from the originals.

# THIS EXTENSION IS BASED ON versionutils.py IN qiskit-metapackage/docs

from functools import partial

from sphinx.util import logging


logger = logging.getLogger(__name__)

translations_list = [
('en', 'English'),
('bn_BN', 'Bengali'),
('fr_FR', 'French'),
('de_DE', 'German'),
('ja_JP', 'Japanese'),
('ko_KR', 'Korean'),
('pt_UN', 'Portuguese'),
('es_UN', 'Spanish'),
('ta_IN', 'Tamil'),
]

default_language = 'en'


def setup(app):
app.connect('config-inited', _extend_html_context)
app.add_config_value('content_prefix', '', '')
app.add_config_value('translations', True, 'html')
return dict(parallel_read_safe=True)
app.add_config_value("content_prefix", default="", rebuild="", types=[str])
app.add_config_value("translations_list", default=[], rebuild="html", types=[list])


def _extend_html_context(app, config):
context = config.html_context
context['translations'] = config.translations
context['translations_list'] = translations_list
context['current_translation'] = _get_current_translation(config) or config.language
Copy link
Member

Choose a reason for hiding this comment

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

Do you know if context['current_translation'] is now used else where?

It was added, IIRC, for the page to remember the language the user was on, when they go from one page to another.

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 couldn't find anywhere it's used. html_context exists so that HTML templates can reference values that are inserted by Python.

Our templates are using language_label instead. If you look at the Python implementation, language_label and current_translation are computed almost the same way.

Copy link
Collaborator

Choose a reason for hiding this comment

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

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 indeed set by Qiskit and all the ecosystem projects. But it is never consumed by anything. We're setting a value for no benefit, which is confusing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah ok, let's remove it for now but make a mental note if we find something breaks for someones project we might need to add it back

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good!

context['translations_list'] = config.translations_list
context['translation_url'] = partial(_get_translation_url, config)
context['language_label'] = _get_language_label(config)


def _get_current_translation(config):
language = config.language or default_language
try:
found = next(v for k, v in translations_list if k == language)
found = next(v for k, v in config.translations_list if k == language)
except StopIteration:
found = None
return found
Expand Down
Loading