Add type hints for ExternalSource, Python Functions, TFRecord Reader, math module #5118
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
klecki
force-pushed
the
static_api_followup
branch
from
850a217
to
28aa6af
Compare
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com> Rework the order of stub generation Split the ops into categories, first do the reimports of python wrappers, than do the generated ones. TODO(klecki): Revert the broken `__module__`? I need the access to the source of the implementation Signed-off-by: Krzysztof Lecki <klecki@nvidia.com> WIP generation Signed-off-by: Krzysztof Lecki <klecki@nvidia.com> Write the stub files by hand Signed-off-by: Krzysztof Lecki <klecki@nvidia.com> Compose - we don;t have anything to constraint as Operetor parent class Signed-off-by: Krzysztof Lecki <klecki@nvidia.com> Add type hints for TFRecord related types Signed-off-by: Krzysztof Lecki <klecki@nvidia.com> Add test with external source Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
klecki
force-pushed
the
static_api_followup
branch
from
352ba6a
to
eb14b30
Compare
klecki
force-pushed
the
static_api_followup
branch
from
cf92e53
to
18096ca
Compare
|
!build |
|
CI MESSAGE: [10453001]: BUILD STARTED |
|
CI MESSAGE: [10453001]: BUILD PASSED |
stiepan
reviewed
Oct 31, 2023
stiepan
reviewed
Oct 31, 2023
stiepan
reviewed
Oct 31, 2023
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
stiepan
reviewed
Nov 3, 2023
stiepan
reviewed
Nov 3, 2023
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
|
!build |
|
CI MESSAGE: [10577941]: BUILD STARTED |
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
|
!build |
|
CI MESSAGE: [10578554]: BUILD STARTED |
|
CI MESSAGE: [10577941]: BUILD PASSED |
stiepan
approved these changes
Nov 3, 2023
stiepan
reviewed
Nov 3, 2023
| @@ -83,8 +82,8 @@ class DLTensorPythonFunction: | |||
| /, | |||
| *, | |||
| function: Optional[Callable[..., Union[Any, Sequence[Any]]]] = None, | |||
| batch_processing: Optional[bool] = False, | |||
| num_outputs: Optional[int] = 1, | |||
| batch_processing: bool = False, | |||
stiepan
reviewed
Nov 3, 2023
| @@ -100,14 +99,13 @@ class DLTensorPythonFunction: | |||
| /, | |||
| *input: DataNode, | |||
| function: Optional[Callable[..., Union[Any, Sequence[Any]]]] = None, | |||
| batch_processing: Optional[bool] = False, | |||
| num_outputs: Optional[int] = 1, | |||
| batch_processing: bool = False, | |||
There was a problem hiding this comment.
I removed those arguments, as I don't think they will work correctly.
We need to consider revisiting the custom implementations for ops, as they do not follow the "pass any argument anywhere" flow.
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
|
!build |
|
CI MESSAGE: [10579585]: BUILD STARTED |
stiepan
approved these changes
Nov 3, 2023
banasraf
approved these changes
Nov 3, 2023
|
CI MESSAGE: [10579585]: BUILD PASSED |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Category: New feature
Description:
Add annotations to operators with custom Python wrappers:
Add stub file for nvidia.dali.tfrecord module.
Add return type annotations to the Pipeline.run function.
The TensorList types again are not fully visible due to being generated
by backend at runtime, next step should provide a dedicated stub file
or alternative implementation.
The stubs are based on the output of mypy
stubgenand thenvidia.dali.ops._signatures._gen_[fn/ops]_signature.The stub generation is reworked, first grouping the operators into
4 categories, so the generated stubs contain imports first.
We utilize the fact that the operators with custom Python wrappers
have dedicated implementation modules now, and reexpose them
in the stubs, allowing the type hints to be picked up.
The external source has non-trivial defaults, expressed mostly via
Noneand cross-dependent on other parameters, so creating a meaningful
annotations there is hard.
There are two overloads provided for the
external_sourcefunction,allowing to disambiguate between the single and multiple outputs
(when
num_outputsparameter was used, and the return type isalways a tuple). Such distinction can't be easily made in the ops API.
TODO:
a lot of code movement to accommodate for the module structure.
Additional information:
Affected modules and functionalities:
Several operators, Pipeline.run, stub generation.
Key points relevant for the review:
If the annotations are ok.
Tests:
Whole DALI must work
Trivial mypy test with external source.
Checklist
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: N/A