jbms / sphinx-immaterial

Adaptation of the popular mkdocs-material material design theme to the sphinx documentation system
https://jbms.github.io/sphinx-immaterial/
Other
177 stars 28 forks source link

satisfy warning about TypedDict #267

Closed 2bndy5 closed 1 year ago

2bndy5 commented 1 year ago

Not sure what instigated this warning, but it reads:

/opt/hostedtoolcache/Python/3.11.3/x64/lib/python3.11/site-packages/sphinx_immaterial/apidoc/cpp/external_cpp_references.py:docstring of sphinx_immaterial.apidoc.cpp.external_cpp_references.ExternalCppReference:1: WARNING: py:class reference target not found: typing_extensions.TypedDict

I think there were some changes to typing-extensions about typing-extensions.TypedDict, but I don't see why we can't just use the implementation available in native python (v3.8 or newer).

2bndy5 commented 1 year ago

The warning seems like an intersphinx problem, and we can now add the typing-extensions intersphinx inventory.

intersphinx_mapping = {
    "python": ("https://docs.python.org/3", None),
    "sphinx_docs": ("https://www.sphinx-doc.org/en/master", None),
    "MyST parser docs": ("https://myst-parser.readthedocs.io/en/latest", None),
    "typing-extensions": ("https://typing-extensions.readthedocs.io/en/latest", None),
}

Although this isn't really needed for our docs with the changes made here. We could conditionally add this inventory for builds with older versions of python. I'm not sure the usefulness of it though.

It would be better to only depend on typing-extensions for when it is needed. I think it was originally added for using the Literal type annotation in python v3.7.

jbms commented 1 year ago

We now actually require Python 3.8 so we can just always use typing.TypedDict.

2bndy5 commented 1 year ago

Most of our other typing-extensions usage is for importing Literal, but there's one import of NotRequired which is now available in python v3.11.

I'm going to re-work this PR to factor out the dependency where possible.

2bndy5 commented 1 year ago

Done. This is needed to get CI passing again. The warning stated in OP is from the CI build logs for #266.

jbms commented 1 year ago

Thanks!