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

pyproject.toml: require Sphinx>=2.0.0 for doc feature #1257

Conversation

Bastian-Krause
Copy link
Member

@Bastian-Krause Bastian-Krause commented Aug 8, 2023

Description
readthedocs installs Python package dependencies like this:

$ pip install --upgrade --no-cache-dir pip setuptools
$ pip install --upgrade --no-cache-dir pillow mock==1.0.1 alabaster>=0.7,<0.8,!=0.7.5 commonmark==0.9.1 recommonmark==0.5.0 sphinx<2 sphinx-rtd-theme<0.5 readthedocs-sphinx-ext<2.3 jinja2<3.1.0
$ pip install --upgrade --upgrade-strategy only-if-needed --no-cache-dir .[doc]

Until recently --upgrade-strategy was specified as eager, but now only-if-needed [1]. This results in Sphinx 1.8.6 being used.

This becomes an issue since Sphinx is not required directly by labgrid, but via sphinx_rtd_theme which loosely requires sphinx >=1.6,<8 [2]. This results in this readthedocs build error:

Exception occurred:
  File "[...]/labgrid/venv-rtd-debugging/lib/python3.11/site-packages/sphinx/ext/autodoc/__init__.py", line 82, in members_option
    return [x.strip() for x in arg.split(',')]
                               ^^^^^^^^^
AttributeError: 'bool' object has no attribute 'split'

We could make this work with Sphinx<2.0.0 [3]:

--- a/doc/conf.py
+++ b/doc/conf.py
@@ -179,7 +179,7 @@ texinfo_documents = [

 autodoc_member_order = 'bysource'
 autodoc_default_options = {
-        'special-members': True,
+        'special-members': None,
 }
 autodoc_mock_imports = ['onewire',
                         'txaio',

But even with this, another error and several warnings occur:

WARNING: Sphinx 1.x is deprecated with sphinx_rtd_theme, update to Sphinx 2.x or greater
WARNING: 'html4_writer' is deprecated with sphinx_rtd_theme

[...]/labgrid/venv-rtd-debugging/lib/python3.11/site-packages/sphinx/util/nodes.py:94: FutureWarning:
   The iterable returned by Node.traverse()
   will become an iterator instead of a list in Docutils > 0.16.
  for classifier in reversed(node.parent.traverse(nodes.classifier)):
[...]/labgrid/doc/getting_started.rst:393: WARNING: Error in "code-block" directive:
1 argument(s) required, 0 supplied.
Exception occurred:
  File "[...]/labgrid/venv-rtd-debugging/lib/python3.11/site-packages/sphinx/ext/napoleon/docstring.py", line 123, in __init__
    elif isinstance(obj, collections.Callable):  # type: ignore
                         ^^^^^^^^^^^^^^^^^^^^
AttributeError: module 'collections' has no attribute 'Callable'

It's not worth the effort to keep this backwards compatible, so simply require Sphinx>=2.0.0 in labgrid directly.

[1] readthedocs/readthedocs.org#10560
[2] https://github.com/readthedocs/sphinx_rtd_theme/blob/b5833585b25358be94918f13c50530e3e9237e7e/setup.cfg
[3] sphinx-doc/sphinx#5459

Checklist

  • PR has been tested

readthedocs installs Python package dependencies like this:

  pip install --upgrade --no-cache-dir pip setuptools
  pip install --upgrade --no-cache-dir pillow mock==1.0.1 alabaster>=0.7,<0.8,!=0.7.5 commonmark==0.9.1 recommonmark==0.5.0 sphinx<2 sphinx-rtd-theme<0.5 readthedocs-sphinx-ext<2.3 jinja2<3.1.0
  pip install --upgrade --upgrade-strategy only-if-needed --no-cache-dir .[doc]

Until recently --upgrade-strategy was specified as `eager`, but now
`only-if-needed` [1]. This results in Sphinx 1.8.6 being used.

This becomes an issue since Sphinx is not required directly by labgrid,
but via sphinx_rtd_theme which loosely requires `sphinx >=1.6,<8` [2].
This results in this readthedocs build error:

  Exception occurred:
    File "[...]/labgrid/venv-rtd-debugging/lib/python3.11/site-packages/sphinx/ext/autodoc/__init__.py", line 82, in members_option
      return [x.strip() for x in arg.split(',')]
                                 ^^^^^^^^^
  AttributeError: 'bool' object has no attribute 'split'

We could make this work with Sphinx<2.0.0 [3]:

  --- a/doc/conf.py
  +++ b/doc/conf.py
  @@ -179,7 +179,7 @@ texinfo_documents = [

   autodoc_member_order = 'bysource'
   autodoc_default_options = {
  -        'special-members': True,
  +        'special-members': None,
   }
   autodoc_mock_imports = ['onewire',
                           'txaio',

But even with this, another errors and several warnings occur:

  WARNING: Sphinx 1.x is deprecated with sphinx_rtd_theme, update to Sphinx 2.x or greater
  WARNING: 'html4_writer' is deprecated with sphinx_rtd_theme

  [...]/labgrid/venv-rtd-debugging/lib/python3.11/site-packages/sphinx/util/nodes.py:94: FutureWarning:
     The iterable returned by Node.traverse()
     will become an iterator instead of a list in Docutils > 0.16.
    for classifier in reversed(node.parent.traverse(nodes.classifier)):
  [...]/labgrid/doc/getting_started.rst:393: WARNING: Error in "code-block" directive:
  1 argument(s) required, 0 supplied.
  Exception occurred:
    File "[...]/labgrid/venv-rtd-debugging/lib/python3.11/site-packages/sphinx/ext/napoleon/docstring.py", line 123, in __init__
      elif isinstance(obj, collections.Callable):  # type: ignore
                           ^^^^^^^^^^^^^^^^^^^^
  AttributeError: module 'collections' has no attribute 'Callable'

It's not worth the effort to keep this backwards compatible, so simply
require Sphinx>=2.0.0 in labgrid directly.

[1] readthedocs/readthedocs.org#10560
[2] https://github.com/readthedocs/sphinx_rtd_theme/blob/b5833585b25358be94918f13c50530e3e9237e7e/setup.cfg
[3] sphinx-doc/sphinx#5459

Signed-off-by: Bastian Krause <[email protected]>
@Bastian-Krause
Copy link
Member Author

@codecov
Copy link

codecov bot commented Aug 8, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.1% ⚠️

Comparison is base (2bfba1a) 62.9% compared to head (3517a22) 62.9%.

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #1257     +/-   ##
========================================
- Coverage    62.9%   62.9%   -0.1%     
========================================
  Files         161     161             
  Lines       11861   11861             
========================================
- Hits         7470    7467      -3     
- Misses       4391    4394      +3     

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Bastian-Krause Bastian-Krause merged commit 1052d34 into labgrid-project:master Aug 10, 2023
8 of 9 checks passed
@Bastian-Krause Bastian-Krause deleted the bst/rtd-sphinx-build-errors branch August 10, 2023 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants