test(core): add cuda.core.__all__ vs public docs consistency check#2347
test(core): add cuda.core.__all__ vs public docs consistency check#2347aryanputta wants to merge 2 commits into
Conversation
Closes NVIDIA#2326. Parses docs/source/api.rst (autosummary entries and data directives while cuda.core is the active module) and compares the flat public names against cuda.core.__all__ in both directions. Dotted entries such as graph.Graph or checkpoint.Process are submodule namespaces and are excluded. Symbols documented in api_private.rst are accepted as documented so returned-helper docs do not fail the check. The tests skip when cuda.core.__all__ is not defined, so this lands independently of NVIDIA#2300 and activates once NVIDIA#2300 merges. Also adds the __all__-names-resolve guard suggested in the NVIDIA#2300 review. Signed-off-by: Aryan <aryansputta@gmail.com>
github-actions
Bot
added
the
cuda.core
@mdboom, there was an error processing your request: See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/1/ |
|
/ok to test 1fc21b2 |
|
There was a problem hiding this comment.
The main change required is that the scope of this is too limited.
It would also be worthwhile verifying that this tests what we think it does by (in another PR) adding some changes that this is expected to catch to confirm that they are in fact caught.
I also think this should land the __all__ into cuda.core here (again, to confirm it's working) rather than waiting for #2300 to add it.
| are accepted as documented. Dotted entries such as ``graph.Graph`` or | ||
| ``checkpoint.Process`` describe submodule namespaces, not the flat | ||
| ``cuda.core`` namespace, and are excluded from the comparison. |
There was a problem hiding this comment.
These should be included (as well as cuda.core.system).
Any __init__.py inside of a public (non _-prefixed) submodule is public API. Currently, graph, system, texture and utils, though more may be added in the future so the test should adapt automatically.
Today, only system has its own separate API .rst page (which is fine). I would argue the other submodules should have their own .rst page as well. (And we could do that refactor here).
Can you assign this to me, but will work on this today. |
…bpackages Addresses review feedback that the consistency check was too narrow: - Define cuda.core.__all__ (flat public namespace) so the check runs instead of skipping, and add an aggregated __all__ to cuda.core.graph derived from its star-imported submodules. - Auto-discover public subpackages from cuda.core.__path__ (graph, system, texture, utils, and any added later; the internal cuNN wheel shims are excluded) and assert each defines a fully resolvable __all__. - Cross-check each documented subpackage's __all__ against api.rst, handling both the dotted (graph.Graph) and flat (currentmodule) doc conventions. system is documented in api_nvml.rst, so its doc cross-check is skipped.
|
Hello @mdboom, thank you for the review. I have addressed both points.
On confirming the check actually catches regressions: I prepared a separate branch that injects one deliberate discrepancy (a symbol documented in |
2 participants
Description
Closes #2326.
Adds
cuda_core/tests/test_api_docs_consistency.py, the follow-on consistency check requested in the #2300 review.What it checks
test_all_exports_resolve: every name incuda.core.__all__resolves viahasattr(the stale-name guard suggested in the review).test_public_symbols_are_documented: fails with "public bycuda.core.__all__but missing from public docs (api.rst)" plus the offending names.test_documented_symbols_are_exported: fails with "documented as public in api.rst but not exported bycuda.core.__all__" plus the offending names.How the docs are parsed
.. module::and.. currentmodule::directives are tracked, so onlyautosummaryentries and.. data::directives active undercuda.corecount. Entries undercuda.core.utilsorcuda.core.textureare excluded automatically.graph.Graphorcheckpoint.Processare submodule namespaces, not flatcuda.corenames, and are excluded.api_private.rstare accepted as documented by their trailing name, so returned helpers such as_device_resources.DeviceResourcesdo not fail the public-docs direction.Ordering with #2300
The tests skip when
cuda.core.__all__is not defined, so this is safe to land before #2300 and activates once #2300 merges. The docs fixture also skips when docs sources are unavailable (installed-wheel test runs).Validation
I do not have a CUDA-capable machine available, so I validated with a stubbed
cuda.coremodule against the real RST files in this repo:__all__set to the list from Add griffe API checking to cuda_core #2300: all 3 tests pass (the currentapi.rst/api_private.rstand the Add griffe API checking to cuda_core #2300__all__are exactly consistent).__all__(current main): all 3 tests skip.__all__and adding a bogus name each fail the corresponding direction with the intended message.ruff checkandruff formatpass.