Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve op deprecation and deprecate sequence reader #5372

Merged
merged 8 commits into from
Mar 15, 2024

Conversation

klecki
Copy link
Contributor

@klecki klecki commented Mar 13, 2024

Category: Other, Refactoring

Description:

Refactor the deprecation warnings so that it shows the proper, fully qualified operator name
and suggests the replacement operator in the fn API (with fully qualified name as well).

Adjust warning and doc generation to allow for not specifying the replacement operator.

Add the deprecation message to the fn.readers.sequence.

Fix the module and display name propagation for external source.

Additional information:

Affected modules and functionalities:

Key points relevant for the review:

Tests:

  • Existing tests apply
  • New tests added
    • Python tests
    • GTests
    • Benchmark
    • Other
  • N/A

Checklist

Documentation

  • Existing documentation applies
  • Documentation updated
    • Docstring
    • Doxygen
    • RST
    • Jupyter
    • Other
  • N/A

DALI team only

Requirements

  • Implements new requirements
  • Affects existing requirements
  • N/A

REQ IDs: N/A

JIRA TASK: N/A

Adjust warning and doc generation to allow for not specyfining the
replacement operator.

Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Copy link
Contributor Author

klecki commented Mar 13, 2024

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [13489282]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [13489282]: BUILD FAILED

Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Copy link
Contributor Author

klecki commented Mar 14, 2024

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [13522094]: BUILD STARTED

This operator is an analogue of VideoReader working on video frames extracted as separate images.
It's main purpose is for test baseline. For regular usage, the VideoReader is
This operator is an analogue of video reader working on video frames extracted as separate images.
It's main purpose is for test baseline. For regular usage, the video reader is
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's not yours, but

Suggested change
It's main purpose is for test baseline. For regular usage, the video reader is
Its main purpose is for test baseline. For regular usage, the video reader is

@stiepan
Copy link
Member

stiepan commented Mar 14, 2024

nitpick: Should we test that? Like the newly depracated operator and maybe some other example with "in_favour_of"? There's even the assert_warns helper somewhere it test utils.

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [13522094]: BUILD FAILED

Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki klecki changed the title Deprecate Sequence Reader. Improve op deprecation and deprecate sequence reader Mar 14, 2024
@klecki
Copy link
Contributor Author

klecki commented Mar 14, 2024

nitpick: Should we test that? Like the newly depracated operator and maybe some other example with "in_favour_of"? There's even the assert_warns helper somewhere it test utils.

I added generic test for this, should I also add the specific one for this operator?

Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Copy link
Contributor Author

klecki commented Mar 14, 2024

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [13533890]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [13533890]: BUILD FAILED

Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Copy link
Contributor Author

klecki commented Mar 15, 2024

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [13549970]: BUILD STARTED

@stiepan
Copy link
Member

stiepan commented Mar 15, 2024

nitpick: Should we test that? Like the newly depracated operator and maybe some other example with "in_favour_of"? There's even the assert_warns helper somewhere it test utils.

I added generic test for this, should I also add the specific one for this operator?

Up to you:)

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [13549970]: BUILD PASSED

@klecki klecki merged commit e43369f into NVIDIA:main Mar 15, 2024
7 checks passed
@klecki klecki deleted the deprecate-sequence-reader branch March 15, 2024 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants