Skip to content

Conversation

@pradyunsg
Copy link
Contributor

Each commit contains a detailed description of the changes made.

This makes the configuration easier to maintain and also makes the documentation's navigation behave more like other Furo-based documentation sets (notably, not duplicating the toc in both the sidebars and showing the current position within the docs along with ability to move around in the docs).

Keeping this as a draft since I wanna see this build on RtD and also check what I need to do in terms of a changelog entry here; and for any feedback around reviewing these changes.

Notably, let me know if you'd prefer that I split the final commit into atomic-ish commits (i.e. one logical change in each commit like... moving to html_css_files in a separate commit, changes to how contents/index/root_doc is handled in a separate commit and so on).

@pradyunsg
Copy link
Contributor Author

IIUC, this should get a skip news.

@pradyunsg pradyunsg marked this pull request as ready for review July 7, 2024 12:39
@nicoddemus nicoddemus added the skip news used on prs to opt out of the changelog requirement label Jul 7, 2024
@pradyunsg
Copy link
Contributor Author

pradyunsg commented Jul 7, 2024

Finally, here's the diff of the old conf.py's vars vs the new conf.py's vars...1

diff --git a/tmp/old.txt b/tmp/new.txt
index 5d548e277..71593df7a 100644
--- a/tmp/old.txt
+++ b/tmp/new.txt
@@ -5,7 +5,7 @@ TYPE_CHECKING -> False
 __annotations__ -> {}
 __builtins__ -> <module 'builtins' (built-in)>
 __doc__ -> None
-__loader__ -> <_frozen_importlib_external.SourceFileLoader object at 0x100b884d0>
+__loader__ -> <_frozen_importlib_external.SourceFileLoader object at 0x1027ac4d0>
 __name__ -> '__main__'
 __package__ -> None
 __spec__ -> None
@@ -15,7 +15,7 @@ autodoc_member_order -> 'bysource'
 autodoc_typehints -> 'description'
 autodoc_typehints_description_target -> 'documented'
 copyright -> '2015, holger krekel and pytest-dev team'
-dedent -> <function dedent at 0x100fa3060>
+dedent -> <function dedent at 0x102bc7060>
 default_role -> 'literal'
 epub_author -> 'holger krekel at merlinux eu'
 epub_copyright -> '2013, holger krekel et alii'
@@ -25,13 +25,14 @@ exclude_patterns -> ['_build', 'naming20.rst', 'test/*', 'old_*', '*attic*', '*/
 extensions -> ['pygments_pytest', 'sphinx.ext.autodoc', 'sphinx.ext.autosummary', 'sphinx.ext.intersphinx', 'sphinx.ext.todo', 'sphinx.ext.viewcode', 'sphinx_removed_in', 'sphinxcontrib_trio', 'sphinxcontrib.towncrier.ext', 'sphinx_issues']
 full_version -> '8.3.0.dev242+g352576a89.d20240707'
 html_baseurl -> 'https://docs.pytest.org/en/stable/'
-html_domain_indices -> True
+html_css_files -> ['pytest-custom.css']
 html_favicon -> 'img/favicon.png'
-html_logo -> 'img/pytest_logo_curves.svg'
+html_logo -> '_static/pytest1.png'
 html_short_title -> 'pytest-8.3'
 html_show_sourcelink -> False
-html_sidebars -> {'index': ['sidebar/brand.html', 'sidebar/search.html', 'sidebar/scroll-start.html', 'sidebarintro.html', 'globaltoc.html', 'links.html', 'sidebar/scroll-end.html', 'style.html'], '**': ['sidebar/brand.html', 'sidebar/search.html', 'sidebar/scroll-start.html', 'globaltoc.html', 'relations.html', 'links.html', 'sidebar/scroll-end.html', 'style.html']}
+html_static_path -> ['_static']
 html_theme -> 'furo'
+html_theme_options -> {'sidebar_hide_name': True}
 html_title -> 'pytest documentation'
 html_use_index -> False
 htmlhelp_basename -> 'pytestdoc'
@@ -44,20 +45,19 @@ latex_engine -> 'lualatex'
 linkcheck_ignore -> ['https://blogs.msdn.microsoft.com/bharry/2017/06/28/testing-in-a-cloud-delivery-cadence/', 'http://pythontesting.net/framework/pytest-introduction/', 'https://github.com/pytest-dev/pytest/issues/\\d+', 'https://github.com/pytest-dev/pytest/pull/\\d+']
 linkcheck_workers -> 5
 man_pages -> [('how-to/usage', 'pytest', 'pytest usage', ['holger krekel at merlinux eu'], 1)]
-master_doc -> 'contents'
 nitpick_ignore -> [('py:class', 'HookCaller'), ('py:class', 'HookspecMarker'), ('py:exc', 'PluginValidationError'), ('py:class', 'ExceptionRepr'), ('py:class', 'Exit'), ('py:class', 'SubRequest'), ('py:class', 'SubRequest'), ('py:class', 'TerminalReporter'), ('py:class', '_pytest._code.code.TerminalRepr'), ('py:class', 'TerminalRepr'), ('py:class', '_pytest.fixtures.FixtureFunctionMarker'), ('py:class', '_pytest.logging.LogCaptureHandler'), ('py:class', '_pytest.mark.structures.ParameterSet'), ('py:class', '_pytest._code.code.Traceback'), ('py:class', '_pytest._py.path.LocalPath'), ('py:class', '_pytest.capture.CaptureResult'), ('py:class', '_pytest.compat.NotSetType'), ('py:class', '_pytest.python.PyCollector'), ('py:class', '_pytest.python.PyobjMixin'), ('py:class', '_pytest.python_api.RaisesContext'), ('py:class', '_pytest.recwarn.WarningsChecker'), ('py:class', '_pytest.reports.BaseReport'), ('py:class', '_tracing.TagTracerSub'), ('py:class', 'warnings.WarningMessage'), ('py:class', 'LEGACY_PATH'), ('py:class', '_PluggyPlugin'), ('py:class', '_pytest._code.code.E'), ('py:class', 'E'), ('py:class', '_pytest.fixtures.FixtureFunction'), ('py:class', '_pytest.nodes._NodeType'), ('py:class', '_NodeType'), ('py:class', '_pytest.python_api.E'), ('py:class', '_pytest.recwarn.T'), ('py:class', '_pytest.runner.TResult'), ('py:obj', '_pytest.fixtures.FixtureValue'), ('py:obj', '_pytest.stash.T'), ('py:class', '_ScopeName')]
 nitpicky -> True
 os -> <module 'os' (frozen)>
 project -> 'pytest'
 release -> '8.3'
-setup -> <function setup at 0x100b623e0>
+root_doc -> 'index'
+setup -> <function setup at 0x1027863e0>
 shutil -> <module 'shutil' from '/Users/pradyunsg/.asdf/installs/python/3.12.4/lib/python3.12/shutil.py'>
-source_suffix -> '.rst'
 templates_path -> ['_templates']
-texinfo_documents -> [('contents', 'pytest', 'pytest Documentation', 'Holger Krekel@*Benjamin Peterson@*Ronny Pfannschmidt@*Floris Bruynooghe@*others', 'pytest', 'simple powerful testing with Python', 'Programming', 1)]
-todo_include_todos -> 1
+texinfo_documents -> [('index', 'pytest', 'pytest Documentation', 'Holger Krekel@*Benjamin Peterson@*Ronny Pfannschmidt@*Floris Bruynooghe@*others', 'pytest', 'simple powerful testing with Python', 'Programming', 1)]
+todo_include_todos -> True
 towncrier_draft_autoversion_mode -> 'draft'
 towncrier_draft_config_path -> 'pyproject.toml'
 towncrier_draft_include_empty -> True
 towncrier_draft_working_directory -> PosixPath('/Users/pradyunsg/Developer/github/pytest')
-version -> '8.3.0.dev242'
+version -> '8.3.0.dev242'

Footnotes

  1. The diff is between the two files, each generated with the below steps run for the two separate commits (main and this PR's HEAD right now).

    • python -i docs/en/conf.py
    • for key, value in sorted(vars().items(), key=lambda x: x[0]): print(key, "->", repr(value)) within that
    • copying the output into a temporary file.

Copy link
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

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

Thanks, it's great to be up to date here.

(I will do a force push which rearranges the commits so we can maintain them in the history)

pradyunsg added 3 commits July 9, 2024 21:59
This change brings the configuration closer with what the latest
sphinx-quickstart generates. It also makes it easier to read through the
documentation configuration file as well.

- Remove unset configuration options
- Drop configuration options that are set to their default values
- Remove placeholder comments, including inline documentation of options
- Group configuration options into sections with links to detailed docs
- Move all custom logic toward the end of the file
This file is not used with Furo.
Move away from injecting unstyled HTML into the sidebar, to using
Furo's default sidebar with Sphinx's `doctree` instead.

This also includes moving to a more typical Sphinx documentation
structure with the `index` page serving as the "root" of the `doctree`
for Sphinx.

Additionally, move custom stylesheets into a `pytest-custom.css` file
and use standard Sphinx tooling to inject these styles.
@bluetech bluetech force-pushed the improve-documentation-configuration branch from 7ac71dd to cc9bf00 Compare July 9, 2024 19:03
@bluetech
Copy link
Member

bluetech commented Jul 9, 2024

I was about to merge, but noticed that in a local build of the docs (tox -e docs) the banner image is broken, pointing to doc/en/_build/html/_static/pytest1.png which doesn't exist.

Also, a pre-existing thing, I noticed that the only way to get to contents.html is to click the banner which seems a bit weird? I expect the banner to go to index.html, but maybe it's just me.

@pradyunsg
Copy link
Contributor Author

It does for me. Can you try again after delete the _build directory?

It's supposed to be the output of copying the doc/en/_static directory (and other stuff) in that folder, so it should certainly be in there.

@pradyunsg
Copy link
Contributor Author

pradyunsg commented Jul 9, 2024

Contents isn't accessible via the sidebar right now (mostly coz the sidebar reflects the site structure at this point) and the logo points to index.html since the initial iteration of this PR. If that's not the case, I'd appreciate more detail on how to reproduce what you're seeing (basically, instructions on what you did from a clean-ish checkout ALA no .tox or _build).

I think you're describing a stale Sphinx output but that might just be me projecting what I see in $day-job onto you. 🙈

@bluetech
Copy link
Member

@pradyunsg Indeed, not sure what happened but after a clean build everything is fine.

@bluetech bluetech merged commit 16cdacc into pytest-dev:main Jul 10, 2024
@nicoddemus
Copy link
Member

Thanks everyone!

I suppose we want to backport?

@pradyunsg pradyunsg deleted the improve-documentation-configuration branch July 10, 2024 19:38
@pradyunsg
Copy link
Contributor Author

I suppose we want to backport?

I think it would be a good idea.

@patchback
Copy link

patchback bot commented Jul 25, 2024

Backport to 8.3.x: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 16cdacc on top of patchback/backports/8.3.x/16cdacc5a373984bc22c300d76e633c3a44bcd35/pr-12584

Backporting merged PR #12584 into main

  1. Ensure you have a local repo clone of your fork. Unless you cloned it
    from the upstream, this would be your origin remote.
  2. Make sure you have an upstream repo added as a remote too. In these
    instructions you'll refer to it by the name upstream. If you don't
    have it, here's how you can add it:
    $ git remote add upstream https://github.com/pytest-dev/pytest.git
  3. Ensure you have the latest copy of upstream and prepare a branch
    that will hold the backported code:
    $ git fetch upstream
    $ git checkout -b patchback/backports/8.3.x/16cdacc5a373984bc22c300d76e633c3a44bcd35/pr-12584 upstream/8.3.x
  4. Now, cherry-pick PR Improve how Sphinx documentation build is configured #12584 contents into that branch:
    $ git cherry-pick -x 16cdacc5a373984bc22c300d76e633c3a44bcd35
    If it'll yell at you with something like fatal: Commit 16cdacc5a373984bc22c300d76e633c3a44bcd35 is a merge but no -m option was given., add -m 1 as follows instead:
    $ git cherry-pick -m1 -x 16cdacc5a373984bc22c300d76e633c3a44bcd35
  5. At this point, you'll probably encounter some merge conflicts. You must
    resolve them in to preserve the patch from PR Improve how Sphinx documentation build is configured #12584 as close to the
    original as possible.
  6. Push this branch to your fork on GitHub:
    $ git push origin patchback/backports/8.3.x/16cdacc5a373984bc22c300d76e633c3a44bcd35/pr-12584
  7. Create a PR, ensure that the CI is green. If it's not — update it so that
    the tests and any other checks pass. This is it!
    Now relax and wait for the maintainers to process your pull request
    when they have some cycles to do reviews. Don't worry — they'll tell you if
    any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@nicoddemus
Copy link
Member

nicoddemus commented Jul 25, 2024

I think it would be a good idea.

Thanks @pradyunsg.

I tried and it failed due to conflicts, but I then realized that it probably is not necessary anymore given it was merged 2 weeks ago, and since then 8.3 has been released already.

@pradyunsg
Copy link
Contributor Author

Good point! https://docs.pytest.org/en/stable/ includes this PR! :)

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

Labels

skip news used on prs to opt out of the changelog requirement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inject CSS via html_css_files rather than inline styles in the sidebar PR #12326 made the bullet lists in the change log invisible

4 participants