Skip to content

Commit cb663c5

Browse files
authored
[refactor] autodoc_dagster.py -> dagster-sphinx (#15407)
## Summary & Motivation Our custom sphinx extension `autodoc_dagster` currently lives in a single file. This PR breaks it into a separate Python package `dagster-sphinx` and refactors it somewhat. This was necessary due to the growing complexity of the extension due to recent and in-progress docs improvements. This PR also adds the new package to the pyright master env, which introduces `sphinx` and `docutils` dependencies and necessitate a pyright pins update (bouns: developing a sphinx plugin is way more pleasant with pyright/LSP help). ## How I Tested These Changes Built docs.
1 parent e6ed078 commit cb663c5

File tree

12 files changed

+221
-113
lines changed

12 files changed

+221
-113
lines changed

docs/sphinx/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
# You can set these variables from the command line.
66
SPHINXOPTS =
7-
SPHINXBUILD = sphinx-build
7+
SPHINXBUILD = sphinx-build -j auto
88
PAPER =
99
BUILDDIR = _build
1010

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
from typing import List, Optional, Tuple, Type, TypeVar
2+
3+
import docutils.nodes as nodes
4+
from dagster._annotations import is_deprecated, is_public
5+
from sphinx.addnodes import versionmodified
6+
from sphinx.application import Sphinx
7+
from sphinx.environment import BuildEnvironment
8+
from sphinx.ext.autodoc import (
9+
ClassDocumenter,
10+
ObjectMembers,
11+
Options as AutodocOptions,
12+
)
13+
from sphinx.util import logging
14+
from typing_extensions import Literal, TypeAlias
15+
16+
from dagster_sphinx.configurable import ConfigurableDocumenter
17+
18+
logger = logging.getLogger(__name__)
19+
20+
##### Useful links for Sphinx documentation
21+
#
22+
# [Event reference] https://www.sphinx-doc.org/en/master/extdev/appapi.html#sphinx-core-events
23+
# These events are emitted during the build and can be hooked into during the
24+
# build process.
25+
# [autodoc] https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html
26+
# Autodoc is not sphinx itself, but it is the central extension that reads
27+
# docstrings.
28+
# [Sphinx extensions API] https://www.sphinx-doc.org/en/master/extdev/index.html
29+
# Root page for learning about writing extensions.
30+
31+
32+
# See: https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html#docstring-preprocessing
33+
# Autodoc doesn't provide it's own alias.
34+
AutodocObjectType: TypeAlias = Literal[
35+
"module", "class", "exception", "function", "method", "attribute"
36+
]
37+
38+
39+
def record_error(env: BuildEnvironment, message: str) -> None:
40+
"""Record an error in the Sphinx build environment. The error list is
41+
globally available during the build.
42+
"""
43+
logger.info(message)
44+
if not hasattr(env, "dagster_errors"):
45+
setattr(env, "dagster_errors", [])
46+
getattr(env, "dagster_errors").append(message)
47+
48+
49+
# ########################
50+
# ##### CHECKS
51+
# ########################
52+
53+
54+
def check_public_method_has_docstring(env: BuildEnvironment, name: str, obj: object) -> None:
55+
if name != "__init__" and not obj.__doc__:
56+
message = (
57+
f"Docstring not found for {object.__name__}.{name}. "
58+
"All public methods and properties must have docstrings."
59+
)
60+
record_error(env, message)
61+
62+
63+
class DagsterClassDocumenter(ClassDocumenter):
64+
"""Overrides the default autodoc ClassDocumenter to adds some extra options."""
65+
66+
objtype = "class"
67+
68+
def get_object_members(self, want_all: bool) -> Tuple[bool, ObjectMembers]:
69+
_, unfiltered_members = super().get_object_members(want_all)
70+
# Use form `is_public(self.object, attr_name) if possible, because to access a descriptor
71+
# object (returned by e.g. `@staticmethod`) you need to go in through
72+
# `self.object.__dict__`-- the value provided in the member list is _not_ the descriptor!
73+
filtered_members = [
74+
m
75+
for m in unfiltered_members
76+
if m[0] in self.object.__dict__ and is_public(self.object.__dict__[m[0]])
77+
]
78+
for member in filtered_members:
79+
check_public_method_has_docstring(self.env, member[0], member[1])
80+
return False, filtered_members
81+
82+
83+
# This is a hook that will be executed for every processed docstring. It modifies the lines of the
84+
# docstring in place.
85+
def process_docstring(
86+
app: Sphinx,
87+
what: AutodocObjectType,
88+
name: str,
89+
obj: object,
90+
options: AutodocOptions,
91+
lines: List[str],
92+
) -> None:
93+
assert app.env is not None
94+
95+
# Insert a "deprecated" sphinx directive (this is built-in to autodoc) for objects flagged with
96+
# @deprecated.
97+
if is_deprecated(obj):
98+
# Note that these are in reversed order from how they will appear because we insert at the
99+
# front. We insert the <placeholder> string because the directive requires an argument that
100+
# we can't supply (we would have to know the version at which the object was deprecated).
101+
# We discard the "<placeholder>" string in `substitute_deprecated_text`.
102+
for line in ["", ".. deprecated:: <placeholder>"]:
103+
lines.insert(0, line)
104+
105+
106+
T_Node = TypeVar("T_Node", bound=nodes.Node)
107+
108+
109+
def get_child_as(node: nodes.Node, index: int, node_type: Type[T_Node]) -> T_Node:
110+
child = node.children[index]
111+
assert isinstance(
112+
child, node_type
113+
), f"Docutils node not of expected type. Expected `{node_type}`, got `{type(child)}`."
114+
return child
115+
116+
117+
def substitute_deprecated_text(app: Sphinx, doctree: nodes.Element, docname: str) -> None:
118+
# The `.. deprecated::` directive is rendered as a `versionmodified` node.
119+
# Find them all and replace the auto-generated text, which requires a version argument, with a
120+
# plain string "Deprecated".
121+
for node in doctree.findall(versionmodified):
122+
paragraph = get_child_as(node, 0, nodes.paragraph)
123+
inline = get_child_as(paragraph, 0, nodes.inline)
124+
text = get_child_as(inline, 0, nodes.Text)
125+
inline.replace(text, nodes.Text("Deprecated"))
126+
127+
128+
def check_custom_errors(app: Sphinx, exc: Optional[Exception] = None) -> None:
129+
dagster_errors = getattr(app.env, "dagster_errors", [])
130+
if len(dagster_errors) > 0:
131+
for error_msg in dagster_errors:
132+
logger.info(error_msg)
133+
raise Exception(
134+
f"Bulid failed. Found {len(dagster_errors)} violations of docstring requirements."
135+
)
136+
137+
138+
def setup(app):
139+
app.setup_extension("sphinx.ext.autodoc") # Require autodoc extension
140+
app.add_autodocumenter(ConfigurableDocumenter)
141+
# override allows `.. autoclass::` to invoke DagsterClassDocumenter instead of default
142+
app.add_autodocumenter(DagsterClassDocumenter, override=True)
143+
app.connect("autodoc-process-docstring", process_docstring)
144+
app.connect("doctree-resolved", substitute_deprecated_text)
145+
app.connect("build-finished", check_custom_errors)
146+
147+
return {
148+
"version": "0.1",
149+
"parallel_read_safe": True,
150+
"parallel_write_safe": True,
151+
}

docs/sphinx/_ext/autodoc_dagster.py renamed to docs/sphinx/_ext/dagster-sphinx/dagster_sphinx/configurable.py

Lines changed: 2 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
import inspect
22
import json
33
import textwrap
4-
from typing import Any, List, Tuple, Type, Union, cast
4+
from typing import Any, List, Type, Union, cast
55

66
import dagster._check as check
7-
import docutils.nodes
87
from dagster import BoolSource, Field, IntSource, StringSource
9-
from dagster._annotations import is_deprecated, is_public
108
from dagster._config.config_type import (
119
Array,
1210
ConfigScalar,
@@ -23,11 +21,7 @@
2321
)
2422
from dagster._core.definitions.configurable import ConfigurableDefinition
2523
from dagster._serdes import ConfigurableClass
26-
from sphinx.addnodes import versionmodified
27-
from sphinx.ext.autodoc import ClassDocumenter, DataDocumenter, ObjectMembers
28-
from sphinx.util import logging
29-
30-
logger = logging.getLogger(__name__)
24+
from sphinx.ext.autodoc import DataDocumenter
3125

3226

3327
def type_repr(config_type: ConfigType) -> str:
@@ -168,98 +162,3 @@ def add_content(self, more_content) -> None:
168162
self.add_line("", source_name)
169163
# do this call at the bottom so that config schema is first thing in documentation
170164
super().add_content(more_content)
171-
172-
173-
class DagsterClassDocumenter(ClassDocumenter):
174-
"""Overrides the default autodoc ClassDocumenter to adds some extra options."""
175-
176-
objtype = "class"
177-
178-
option_spec = ClassDocumenter.option_spec.copy()
179-
option_spec["deprecated_aliases"] = lambda string: [s.strip() for s in string.split(",")]
180-
181-
def add_content(self, *args, **kwargs):
182-
super().add_content(*args, **kwargs)
183-
source_name = self.get_sourcename()
184-
for alias in self.options.get("deprecated_aliases", []):
185-
self.add_line(f"ALIAS: {alias}", source_name)
186-
187-
def record_error(self, message: str) -> None:
188-
if not hasattr(self.env, "dagster_errors"):
189-
self.env.dagster_errors = []
190-
self.env.dagster_errors.append(message)
191-
192-
def get_object_members(self, want_all: bool) -> Tuple[bool, ObjectMembers]:
193-
_, unfiltered_members = super().get_object_members(want_all)
194-
# Use form `is_public(self.object, attr_name) if possible, because to access a descriptor
195-
# object (returned by e.g. `@staticmethod`) you need to go in through
196-
# `self.object.__dict__`-- the value provided in the member list is _not_ the descriptor!
197-
filtered_members = [
198-
m
199-
for m in unfiltered_members
200-
if (
201-
m[0] in self.object.__dict__
202-
and is_public(self.object.__dict__[m[0]])
203-
or is_public(m[1])
204-
)
205-
]
206-
for member in filtered_members:
207-
if member[0] != "__init__" and not member[1].__doc__:
208-
msg = (
209-
f"Docstring not found for {self.object.__name__}.{member[0]}. "
210-
"All public methods and properties must have docstrings."
211-
)
212-
logger.info(msg)
213-
self.record_error(msg)
214-
return False, filtered_members
215-
216-
217-
# This is a hook that will be executed for every processed docstring. It modifies the lines of the
218-
# docstring in place.
219-
def process_docstring(app, what, name, obj, options, lines):
220-
# Insert a "deprecated" sphinx directive (this is built-in to autodoc) for objects flagged with
221-
# @deprecated.
222-
if is_deprecated(obj):
223-
# Note that these are in reversed order from how they will appear because we insert at the
224-
# front. We insert the <placeholder> string because the directive requires an argument that
225-
# we can't supply (we would have to know the version at which the object was deprecated).
226-
# We discard the "<placeholder>" string in `substitute_deprecated_text`.
227-
for line in ["", ".. deprecated:: <placeholder>"]:
228-
lines.insert(0, line)
229-
230-
231-
def substitute_deprecated_text(app, doctree, fromdocname):
232-
# The `.. deprecated::` directive is rendered as a `versionmodified` node.
233-
# Find them all and replace the auto-generated text, which requires a version argument, with a
234-
# plain string "Deprecated".
235-
for node in doctree.findall(versionmodified):
236-
paragraph = node.children[0]
237-
inline = paragraph.children[0]
238-
text = inline.children[0]
239-
inline.replace(text, docutils.nodes.Text("Deprecated"))
240-
241-
242-
def check_custom_errors(app, _):
243-
if hasattr(app.env, "dagster_errors"):
244-
for error_msg in app.env.dagster_errors:
245-
logger.info(error_msg)
246-
raise Exception(
247-
f"Bulid failed. Found {len(app.env.dagster_errors)} violations of docstring"
248-
" requirements."
249-
)
250-
251-
252-
def setup(app):
253-
app.setup_extension("sphinx.ext.autodoc") # Require autodoc extension
254-
app.add_autodocumenter(ConfigurableDocumenter)
255-
# override allows `.. autoclass::` to invoke DagsterClassDocumenter instead of default
256-
app.add_autodocumenter(DagsterClassDocumenter, override=True)
257-
app.connect("autodoc-process-docstring", process_docstring)
258-
app.connect("doctree-resolved", substitute_deprecated_text)
259-
app.connect("build-finished", check_custom_errors)
260-
261-
return {
262-
"version": "0.1",
263-
"parallel_read_safe": True,
264-
"parallel_write_safe": True,
265-
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
from setuptools import find_packages, setup
2+
3+
setup(
4+
name="dagster-sphinx",
5+
version="0.0.1",
6+
author="Elementl",
7+
author_email="[email protected]",
8+
license="Apache-2.0",
9+
description="Dagster-specific sphinx extension.",
10+
url="https://github.com/dagster-io/dagster",
11+
classifiers=[
12+
"Programming Language :: Python :: 3.8",
13+
"Programming Language :: Python :: 3.9",
14+
"Programming Language :: Python :: 3.10",
15+
"License :: OSI Approved :: Apache Software License",
16+
"Operating System :: OS Independent",
17+
],
18+
packages=find_packages(exclude=["dagster_sphinx_tests*"]),
19+
install_requires=[
20+
"sphinx",
21+
"sphinx_toolbox",
22+
],
23+
)
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
[tox]
2+
skipsdist = true
3+
4+
[testenv]
5+
download = True
6+
passenv = CI_PULL_REQUEST COVERALLS_REPO_TOKEN BUILDKITE*
7+
allowlist_externals =
8+
/bin/bash
9+
commands =
10+
!windows: /bin/bash -c '! pip list --exclude-editable | grep -e dagster'
11+
pytest -c ../../pyproject.toml -vv {posargs}

docs/sphinx/conf.py

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,6 @@
7979
copyright = "2019, Dagster Labs, Inc" # noqa: A001
8080
author = "Dagster Labs"
8181

82-
# The short X.Y version
83-
version = ""
84-
# The full version, including alpha/beta/rc tags
85-
release = ""
86-
8782
# -- General configuration ---------------------------------------------------
8883

8984
# NOTE: `sphinx.ext.*` extensions are built-in to sphinx-- all others are supplied by other
@@ -101,8 +96,8 @@
10196
"sphinx.ext.viewcode",
10297
# Directives for automatically documenting CLIs built with the `click` package.
10398
"sphinx_click.ext",
104-
# Dagster-Labs-authored extension with custom directives and sphinx processing.
105-
"autodoc_dagster",
99+
# Dagster-labs-authored extension with custom directives and sphinx processing.
100+
"dagster_sphinx",
106101
# Renders a collapsible HTML component. Used by autodoc_dagster.
107102
"sphinx_toolbox.collapse",
108103
]

docs/tox.ini

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ deps =
1414
sphinx-click==4.3.0
1515
sphinx_toolbox
1616
sphinxcontrib-serializinghtml<1.1.6 # pin away new version that manifests TypeError
17+
-e sphinx/_ext/dagster-sphinx
1718

1819
# Can't stub deps because processed by sphinx-click
1920
-e ../python_modules/dagster

pyproject.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ target-versions = ['py38', 'py39', 'py310', 'py311']
3737

3838
include = [
3939
".buildkite/dagster-buildkite",
40+
"docs/sphinx/_ext/dagster-sphinx",
4041
"python_modules",
4142
"examples",
4243
"integration_tests",

pyright/alt-1/requirements-pinned.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ pendulum==2.1.2
171171
pexpect==4.8.0
172172
pickleshare==0.7.5
173173
Pillow==10.0.0
174-
platformdirs==3.8.1
174+
platformdirs==3.9.1
175175
pluggy==1.2.0
176176
-e examples/project_fully_featured
177177
prometheus-client==0.17.1

pyright/master/include.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
.buildkite/dagster-buildkite
2+
docs/sphinx/_ext/dagster-sphinx
23
python_modules
34
examples
45
integration_tests

0 commit comments

Comments
 (0)