Dynamic mode operator docstrings #6078

mzient · 2025-10-30T16:04:11Z

Category:

New feature (non-breaking change which adds functionality)
Refactoring (Redesign of existing code that doesn't affect functionality)

Description:

This PR refactors the name and signature generation and uses (to an extent possible) a single source of truth for the names used in different modes. It also adds "dynamic" API to the name-generating functions, adds argument filtering and replaces TensorList with Tensor/Batch for the dynamic API.

Additional information:

Affected modules and functionalities:

Signatures, dynamic mode op_builder, documentation.

Key points relevant for the review:

Tests:

Existing test check for breakage. Documentation is yet another test (both that it builds and looks ok).

Checklist

Documentation

DALI team only

Requirements

Implements new requirements
Affects existing requirements
N/A

REQ IDs: N/A

JIRA TASK: DALI-4464

mzient · 2025-10-30T16:04:50Z

dali/python/nvidia/dali/experimental/dynamic/_op_builder.py

    return function


+def build_fn_wrappers(all_ops):


moved here from the (now removed) _fn.py

dali-automaton · 2025-10-30T16:06:37Z

CI MESSAGE: [37593417]: BUILD STARTED

dali/python/nvidia/dali/experimental/dynamic/ops.py

-    global _all_ops
-    _all_ops = _op_builder.build_operators()
+    global _all_ops, _all_functions
+    _all_ops, _all_functions = _op_builder.build_operators()


dali/python/nvidia/dali/ops/_docs.py

 from nvidia.dali.ops import _registry, _names


+_MAX_INPUT_SPELLED_OUT = 5


greptile-apps

Greptile Overview

Greptile Summary

This PR refactors documentation generation and naming for DALI's dynamic mode API. Key changes include:

Single source of truth for names: Unified naming logic across different API modes (ops, fn, dynamic) in _names.py and _docs.py
Dynamic API docstrings: Added comprehensive docstring generation for dynamic mode operators and functions, with automatic replacement of "TensorList" references with "Tensor/Batch"
Improved input naming: Refactored _get_input_name() to handle name collisions with operator arguments and removed the __ prefix for cleaner signatures
Consolidated function wrappers: Moved function wrapper building from separate _fn.py into _op_builder.py with proper docstring generation
Argument filtering: Added support for filtering which arguments appear in documentation based on what's actually used in dynamic mode
New tests: Added comprehensive tests to verify documentation is present and correctly formatted for dynamic API

The refactoring improves code maintainability by reducing duplication and ensures consistent documentation across all API modes.

Confidence Score: 4/5

This PR is safe to merge with minimal risk, though thorough testing of documentation generation is recommended
The PR is a well-structured refactoring focused on documentation and naming. The changes are mostly additive (adding dynamic API support) with some consolidation (removing _fn.py). The logic changes in _get_input_name() for collision detection are sound, and comprehensive tests were added. Score is 4/5 rather than 5/5 due to the complexity of the signature/docstring generation code and the removal of the __ prefix from input names which could potentially affect downstream tooling, though the changes appear backward compatible for the public APIs.
Pay closer attention to dali/python/nvidia/dali/ops/_names.py for the input name collision logic, and ensure the documentation tests pass in CI

Important Files Changed

File Analysis

Filename	Score	Overview
dali/python/nvidia/dali/types.py	5/5	Added `api` parameter to `_type_name_convert_to_string` to replace "TensorList" with "Tensor/Batch" for dynamic API
dali/python/nvidia/dali/ops/_names.py	4/5	Refactored input name generation with collision detection; added dynamic API support; removed `__` prefix from input names
dali/python/nvidia/dali/ops/_docs.py	5/5	Added dynamic API support with TensorList→Tensor/Batch replacement; added argument filtering; refactored docstring generators
dali/python/nvidia/dali/experimental/dynamic/_op_builder.py	4/5	Major refactor: added docstring generation, consolidated function wrapper building, improved input handling with `_get_inputs()`

Sequence Diagram

sequenceDiagram
    participant User
    participant OpBuilder as _op_builder
    participant Docs as _docs
    participant Names as _names
    participant Types as types
    participant Schema as OpSchema
    
    User->>OpBuilder: build_operators()
    OpBuilder->>OpBuilder: build_constructor(schema, op_class)
    OpBuilder->>Schema: GetArgumentNames()
    Schema-->>OpBuilder: argument list
    OpBuilder->>Docs: _docstring_generator_class(schema_name, api="dynamic", args)
    Docs->>Schema: get operator info
    Docs->>Docs: _get_inputs_doc(schema, api="dynamic")
    Docs->>Names: _get_input_name(schema, input_idx)
    Names->>Schema: HasInputDox(), GetInputName()
    Names->>Schema: HasArgument(name) [collision check]
    Names-->>Docs: input name
    Docs->>Docs: _adjust_dox(dox, api="dynamic")
    Note over Docs: Replace "TensorList" with "Tensor/Batch"
    Docs->>Docs: _get_kwargs(schema, api="dynamic", args)
    Docs->>Types: _type_name_convert_to_string(dtype, allow_tensors, api="dynamic")
    Types-->>Docs: type string with "Tensor/Batch"
    Docs-->>OpBuilder: complete docstring
    OpBuilder->>OpBuilder: makefun.create_function(header, init, doc)
    
    OpBuilder->>OpBuilder: build_call_function(schema, op_class)
    OpBuilder->>Docs: _docstring_generator_call(schema_name, api="dynamic", args)
    Docs->>Docs: Similar flow as above
    Docs-->>OpBuilder: call docstring
    OpBuilder->>OpBuilder: makefun.create_function(header, call, doc)
    
    OpBuilder->>OpBuilder: build_fn_wrappers(all_ops)
    loop for each operator
        OpBuilder->>OpBuilder: build_fn_wrapper(op)
        OpBuilder->>Names: _get_inputs(schema)
        Names->>Names: _get_input_name() with collision detection
        Names-->>OpBuilder: input parameters
        OpBuilder->>Docs: _docstring_generator_fn(schema_name, api="dynamic", args)
        Docs-->>OpBuilder: function docstring
        OpBuilder->>OpBuilder: makefun.create_function(header, fn_call, doc)
    end
    
    OpBuilder-->>User: all_op_classes, all_fn_wrappers

_{11 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

dali-automaton · 2025-10-30T17:53:25Z

CI MESSAGE: [37593417]: BUILD FAILED

szkarpinski · 2025-11-03T11:47:24Z

help(ndd.readers.File) still mentions pipeline mode:

|  __call__(self, *, batch_size=None)
 |      Operator call to be used in graph definition. This operator doesn't have any inputs.

szkarpinski · 2025-11-03T11:48:43Z

help(ndd) mentions internal things as package contents:

NAME
    nvidia.dali.experimental.dynamic - Dynamic API - a new experimental API that allows to interleave DALI operations with Python code.

PACKAGE CONTENTS
    _batch
    _device
    _eval_context
    _eval_mode
    _fn
    _invocation
    _op_builder
    _tensor
    _type
    math
    ops

szkarpinski · 2025-11-03T11:50:53Z

help(ndd.Tensor) and help(ndd.Batch) don't have docstrings describing what they are. Also, docstrings for important user-facing functions like Tensor.{cpu,evaluate,item,to_device} are missing

Signed-off-by: Michal Zientkiewicz <[email protected]>

Signed-off-by: Michał Zientkiewicz <[email protected]>

Signed-off-by: Michal Zientkiewicz <[email protected]>

mzient · 2025-11-03T12:18:16Z

help(ndd.Tensor) and help(ndd.Batch) don't have docstrings describing what they are. Also, docstrings for important user-facing functions like Tensor.{cpu,evaluate,item,to_device} are missing

Those are quite valid points, but the fix is just to write the documentation. This PR is about checking the auto-generated docstrings for operators.

mzient · 2025-11-03T12:19:12Z

help(ndd.readers.File) still mentions pipeline mode:

|  __call__(self, *, batch_size=None)
 |      Operator call to be used in graph definition. This operator doesn't have any inputs.

Fixed.

greptile-apps

Greptile Overview

Greptile Summary

This PR refactors operator documentation generation to support the dynamic mode API with proper docstrings.

Key changes:

Unified name and signature generation across ops, fn, and dynamic APIs using a single source of truth
Added api="dynamic" parameter throughout the documentation stack to replace TensorList references with Tensor/Batch
Improved input name collision detection in _get_input_name() to prevent conflicts with operator arguments
Consolidated function wrapper creation into build_operators() instead of separate initialization
Added schema.Name() method binding in C++ backend for consistent schema name access

Implementation highlights:

The _get_inputs() helper function standardizes positional input parameter generation across constructors and function wrappers
Documentation generators now filter arguments based on used_kwargs to show only relevant parameters
Input names use collision avoidance logic (prepending __ or appending _input) when conflicts with argument names occur
The _MAX_INPUT_SPELLED_OUT constant (5) controls when variadic *inputs is used vs spelling out individual parameters

Testing:
Tests verify that dynamic mode documentation excludes TensorList and graph references, ensuring proper API-specific terminology

Confidence Score: 5/5

This PR is safe to merge with no critical issues found
The refactoring is well-structured with consistent patterns across the codebase. All changes are additive/refactoring with no breaking modifications to existing APIs. The code properly handles edge cases (zero inputs, collision avoidance, optional parameters) and includes comprehensive tests to validate the documentation generation
No files require special attention

Important Files Changed

File Analysis

Filename	Score	Overview
dali/python/nvidia/dali/ops/_names.py	5/5	Refactored input name generation with collision detection and added support for dynamic API naming conventions
dali/python/nvidia/dali/ops/_docs.py	5/5	Extended documentation generators to support dynamic API with Tensor/Batch types and argument filtering
dali/python/nvidia/dali/experimental/dynamic/_op_builder.py	5/5	Integrated docstring generation, refactored signature building with `_get_inputs()`, and consolidated function wrapper creation
dali/test/python/experimental_mode/test_docs.py	5/5	Comprehensive test suite verifying dynamic mode documentation correctness, checking for TensorList/graph references and doc presence

Sequence Diagram

sequenceDiagram
    participant User
    participant DynamicOps as dynamic.ops
    participant OpBuilder as _op_builder
    participant Docs as _docs
    participant Names as _names
    participant Backend as backend_impl

    User->>DynamicOps: Import dynamic module
    DynamicOps->>OpBuilder: _initialize() -> build_operators()
    
    loop For each registered operator
        OpBuilder->>Backend: GetSchema(schema_name)
        Backend-->>OpBuilder: OpSchema
        
        OpBuilder->>OpBuilder: build_constructor(schema, op_class)
        OpBuilder->>Names: _get_input_name(schema, i)
        Names->>Backend: schema.GetInputName() / schema.HasArgument()
        Names-->>OpBuilder: input_name (with collision avoidance)
        
        OpBuilder->>Docs: _docstring_generator_class(schema.Name(), api="dynamic")
        Docs->>Names: _get_input_name(schema, i)
        Docs->>Docs: _adjust_dox(dox, api) - Replace TensorList with Tensor/Batch
        Docs-->>OpBuilder: Constructor docstring
        
        OpBuilder->>OpBuilder: build_call_function(schema, op_class)
        OpBuilder->>Docs: _docstring_generator_call(schema.Name(), api="dynamic")
        Docs-->>OpBuilder: __call__ docstring
        
        OpBuilder->>OpBuilder: build_fn_wrapper(op)
        OpBuilder->>OpBuilder: _get_inputs(schema)
        OpBuilder->>Docs: _docstring_generator_fn(schema.Name(), api="dynamic")
        Docs-->>OpBuilder: Function docstring
    end
    
    OpBuilder-->>DynamicOps: (all_op_classes, all_fn_wrappers)
    DynamicOps-->>User: Operators with dynamic mode docstrings

_{11 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

dali-automaton · 2025-11-03T12:22:24Z

CI MESSAGE: [37806636]: BUILD STARTED

rostan-t · 2025-11-03T12:24:43Z

dali/python/nvidia/dali/ops/_docs.py



-def _docstring_prefix_from_inputs(op_name):
+def _call_description(api):


It's not clear from the docstring what to expect when calling a reader. Can some generic description be filled here or should it go in the class docstring instead (out of the scope of this PR)?

The use of __call__ on operators is going to be rather an exception than a pattern. I think we can work it out later, if necessary.

szkarpinski

In a follow-up, please address the remaining comments. In particular, let's make sure that reader's documentation is helpful

dali-automaton · 2025-11-03T13:40:21Z

CI MESSAGE: [37809283]: BUILD STARTED

dali-automaton · 2025-11-03T15:21:12Z

CI MESSAGE: [37809283]: BUILD PASSED

* Add name generation for dynamic API. * Limit the number of spelled-out arguments. * Adjust argument type name for dynamic API (use Tensor/Batch instead of TensorList) * Add docstrings for dynamic mode ops and functions. * Fix __call__ documentation. Update tests to look for mention of graph. Signed-off-by: Michal Zientkiewicz <[email protected]>

mzient commented Oct 30, 2025

View reviewed changes

github-advanced-security bot found potential problems Oct 30, 2025

View reviewed changes

greptile-apps bot reviewed Oct 30, 2025

View reviewed changes

dali-automaton assigned rostan-t and szkarpinski Oct 31, 2025

mzient changed the title ~~Dynamic mode docstrings~~ Dynamic mode operator docstrings Nov 3, 2025

mzient and others added 8 commits November 3, 2025 13:16

Name generation for dynamic API.

813eecd

Signed-off-by: Michal Zientkiewicz <[email protected]>

[WIP]

bf125bc

Signed-off-by: Michał Zientkiewicz <[email protected]>

[WIP]

1322e29

Signed-off-by: Michal Zientkiewicz <[email protected]>

[WIP]

8afe8c6

Signed-off-by: Michal Zientkiewicz <[email protected]>

Limit the number of spelled-out arguments.

d48c3ab

Signed-off-by: Michal Zientkiewicz <[email protected]>

Add docstrings for dynamic mode ops and functions.

55c2d2c

Signed-off-by: Michal Zientkiewicz <[email protected]>

Cleanup.

adde2fe

Signed-off-by: Michal Zientkiewicz <[email protected]>

Fix __call__ documentation. Update tests to look for mention of graph.

d617549

Signed-off-by: Michal Zientkiewicz <[email protected]>

mzient force-pushed the dynamic_mode_docstrings branch from e00718f to d617549 Compare November 3, 2025 12:16

greptile-apps bot reviewed Nov 3, 2025

View reviewed changes

rostan-t reviewed Nov 3, 2025

View reviewed changes

rostan-t approved these changes Nov 3, 2025

View reviewed changes

szkarpinski approved these changes Nov 3, 2025

View reviewed changes

NVIDIA deleted a comment from dali-automaton Nov 3, 2025

mzient merged commit b7fae95 into NVIDIA:main Nov 3, 2025
6 checks passed

		from nvidia.dali.ops import _registry, _names


		_MAX_INPUT_SPELLED_OUT = 5



		def _docstring_prefix_from_inputs(op_name):
		def _call_description(api):

Dynamic mode operator docstrings #6078

Dynamic mode operator docstrings #6078

Uh oh!

Conversation

mzient commented Oct 30, 2025

Category:

Description:

Additional information:

Affected modules and functionalities:

Key points relevant for the review:

Tests:

Checklist

Documentation

DALI team only

Requirements

Uh oh!

mzient Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

dali-automaton commented Oct 30, 2025

Uh oh!

Uh oh!

Uh oh!

Check notice

Check notice

greptile-apps bot left a comment

Choose a reason for hiding this comment

Greptile Overview

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Uh oh!

dali-automaton commented Oct 30, 2025

Uh oh!

szkarpinski commented Nov 3, 2025

Uh oh!

szkarpinski commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

szkarpinski commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mzient commented Nov 3, 2025

Uh oh!

mzient commented Nov 3, 2025

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Greptile Overview

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

dali-automaton commented Nov 3, 2025

Uh oh!

rostan-t Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

mzient Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

szkarpinski left a comment

Choose a reason for hiding this comment

Uh oh!

dali-automaton commented Nov 3, 2025

Uh oh!

dali-automaton commented Nov 3, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

szkarpinski commented Nov 3, 2025 •

edited

Loading

szkarpinski commented Nov 3, 2025 •

edited

Loading