Skip to content

Comments

swev-id: sphinx-doc__sphinx-9258 - Support '|' union types in Python docfields (:type, :vartype) and add tests#78

Open
casey-brooks wants to merge 2 commits intosphinx-doc__sphinx-9258from
noa/issue-72
Open

swev-id: sphinx-doc__sphinx-9258 - Support '|' union types in Python docfields (:type, :vartype) and add tests#78
casey-brooks wants to merge 2 commits intosphinx-doc__sphinx-9258from
noa/issue-72

Conversation

@casey-brooks
Copy link

@casey-brooks casey-brooks commented Dec 26, 2025

Summary

  • Support union types with the | operator in Python docfields so each operand is linked independently.
  • Add regression coverage for union docfields in parameters and vartype entries.
  • Include a nitpicky test root exercising union docfields for manual reproduction.

Linked Issue

Reproduction (before fix) on base branch (sphinx-doc__sphinx-9258)

  1. Create a minimal project using tests/roots/test-domain-py-union:
    • conf.py: nitpicky = True
    • index.rst:
      .. py:function:: sample(text)
      
         :param text: textual data
         :type text: bytes | str
  2. Run:
    sphinx-build -W -b html tests/roots/test-domain-py-union _build/union-html

Observed failure output (before fix)

  • The entire string bytes | str is treated as a single reference target, which fails to resolve when nitpicky is enabled. Example warning:
WARNING: py:class reference target not found: bytes | str
  • As a result, either a missing-reference warning appears (nitpicky), or the content renders as plain text without independent links.

Fix

  • sphinx/domains/python.py: Extend PyXrefMixin.make_xrefs delimiter regex to split on | as a union delimiter (spaces around the operator).
  • Does not affect existing behavior for or, commas, brackets, or ellipsis.

Tests

  • tests/test_domain_py.py:
    • test_info_field_list_union_operator: verifies bytes | str, legacy str or int or None, nested generics with unions, ellipsis unions, and literal pipe non-splitting.
    • test_info_field_list_vartype_union: verifies vartype union linking.
  • Manual root: tests/roots/test-domain-py-union/ for ad-hoc builds.

Local results

  • Targeted tests pass locally.
  • Full test run in this environment may fail due to external tool availability (e.g., ImageMagick convert) and known typing expectation diffs; this change is isolated to Python domain docfield linking and is covered by dedicated tests above.

@casey-brooks casey-brooks requested a review from a team December 26, 2025 18:58
@casey-brooks
Copy link
Author

Test & Lint Summary

  • PYTHONPATH=/workspace:/workspace/sphinx .venv/bin/pytest tests/test_domain_py.py::test_info_field_list_union_operator tests/test_domain_py.py::test_info_field_list_vartype_union
  • .venv/bin/flake8 sphinx/domains/python.py tests/test_domain_py.py

Full pytest run currently fails in this pinned Sphinx 4.1 / Python 3.10 environment (missing ImageMagick convert and known typing expectation diffs).

@rowan-stein rowan-stein changed the title Fix docfield union docfields linking swev-id: sphinx-doc__sphinx-9258 - Support '|' union types in Python docfields (:type, :vartype) and add tests Dec 26, 2025
Copy link

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

Summary:\n- [major] Tighten the union delimiter detection: still fires inside literals like , so we end up splitting and cross-referencing the literal pieces. Please require whitespace on both sides without breaking spacing and add regression coverage for that case.\n- Please also update to mention the new union support so the docs stay accurate.

@casey-brooks
Copy link
Author

Test & Lint Summary

  • PYTHONPATH=/workspace:/workspace/sphinx .venv/bin/pytest tests/test_domain_py.py::test_info_field_list_union_operator tests/test_domain_py.py::test_info_field_list_vartype_union
  • .venv/bin/flake8 sphinx/domains/python.py tests/test_domain_py.py

Full pytest run remains flaky in this pinned Sphinx 4.1 / Python 3.10 environment (missing ImageMagick convert and known typing expectation diffs).

Copy link

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

Summary:

  • Regex now only treats | as a union delimiter when it has whitespace on both sides, so literals like Literal['foo| bar'] stay intact.
  • Regression tests cover the literal pipe case for both :type and :vartype fields.
  • Documentation updated to mention the PEP 604-style union syntax and behavior for literal pipes.

Looks good to me—thanks!

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

Successfully merging this pull request may close these issues.

2 participants