remove duplicate definition of manifest read and write func. #6088

XuesongYang · 2023-02-23T01:07:13Z

What does this PR do ?

remove duplicate definitions of read and write manifest files, and re-use ones defined in nemo.collections.asr.parts.utils.manifest_utils

Collection: [Note which collection this PR will affect]

Changelog

Add specific line by line info of high level changes in this PR.

Usage

You can potentially add a usage example below

# Add a code snippet demonstrating how to use this

Before your PR is "Ready for review"

Pre checks:

Make sure you read and followed Contributor guidelines
Did you write any new necessary tests?
Did you add or update any necessary documentation?
Does the PR affect components that are optional to install? (Ex: Numba, Pynini, Apex etc)
- Reviewer: Does the PR have correct import guards for all optional libraries?

PR Type:

New Feature
Bugfix
Documentation

If you haven't finished some of the above items you can still open "Draft" PR.

Who can review?

Anyone in the community is free to review the PR once the checks have passed.
Contributor guidelines contains specific people who can review PRs to various areas.

Additional Information

Related to # (issue)

nemo/collections/tts/data/data_utils.py

scripts/dataset_processing/tts/audio_processing/preprocess_audio.py

nemo/collections/tts/data/data_utils.py

scripts/dataset_processing/tts/audio_processing/preprocess_audio.py

racoiaws · 2023-02-24T18:38:42Z

Maybe this could be moved outside of ASR collection then? Somewhere like nemo.utils or nemo.core.utils

XuesongYang · 2023-03-08T21:35:18Z

Maybe this could be moved outside of ASR collection then? Somewhere like nemo.utils or nemo.core.utils

Can't agree more! Maybe we just hold off now until the next massive refactoring. @titu1994

titu1994

Very neat, thanks ! Can you make the minor modification to accepting both path and str to simplify the call to these functions and then merge ?

scripts/dataset_processing/tts/audio_processing/preprocess_audio.py

scripts/dataset_processing/tts/resynthesize_dataset.py

redoctopus

Thanks for this cleanup :) Will wait for CodeQL fixes before approving.

titu1994

Notebooks need to be reverted and updated in colab env, then download the colab one. Simpler to just remove them for now since there's no need to unify notebook manifest reading and writing

docs/source/asr/examples/kinyarwanda_asr.rst

tutorials/asr/asr_adapters/ASR_with_Adapters.ipynb

tutorials/asr/Buffered_Transducer_Inference_with_LCS_Merge.ipynb

Signed-off-by: Xuesong Yang <[email protected]>

for more information, see https://pre-commit.ci

Signed-off-by: Xuesong Yang <[email protected]>

This reverts commit 033d998. Signed-off-by: Xuesong Yang <[email protected]>

Signed-off-by: Xuesong Yang <[email protected]>

reverted notebook and updated functions.

…6088) * unify duplicate definitions of manifest read and write func. * extend support of both Path and str. * update ones in tutorials. --------- Signed-off-by: Xuesong Yang <[email protected]>

…6088) * unify duplicate definitions of manifest read and write func. * extend support of both Path and str. * update ones in tutorials. --------- Signed-off-by: Xuesong Yang <[email protected]> Signed-off-by: hsiehjackson <[email protected]>

github-actions bot added ASR TTS labels Feb 23, 2023

XuesongYang requested review from rlangman, bmwshop, racoiaws and nithinraok February 23, 2023 01:08

XuesongYang commented Feb 23, 2023

View reviewed changes

nemo/collections/tts/data/data_utils.py Outdated Show resolved Hide resolved

github-advanced-security bot found potential problems Feb 23, 2023

View reviewed changes

scripts/dataset_processing/tts/audio_processing/preprocess_audio.py Fixed Show fixed Hide fixed

scripts/dataset_processing/tts/audio_processing/preprocess_audio.py Fixed Show fixed Hide fixed

XuesongYang force-pushed the remove-duplicate-codes branch from a80fd22 to 593fe5e Compare February 23, 2023 02:50

nithinraok reviewed Feb 23, 2023

View reviewed changes

nemo/collections/tts/data/data_utils.py Outdated Show resolved Hide resolved

scripts/dataset_processing/tts/audio_processing/preprocess_audio.py Outdated Show resolved Hide resolved

XuesongYang force-pushed the remove-duplicate-codes branch from 593fe5e to 95af109 Compare March 8, 2023 21:33

XuesongYang force-pushed the remove-duplicate-codes branch from 95af109 to 4bacce4 Compare March 8, 2023 21:38

XuesongYang requested review from nithinraok and redoctopus March 8, 2023 21:40

titu1994 previously approved these changes Mar 9, 2023

View reviewed changes

scripts/dataset_processing/tts/audio_processing/preprocess_audio.py Show resolved Hide resolved

scripts/dataset_processing/tts/resynthesize_dataset.py Outdated Show resolved Hide resolved

redoctopus reviewed Mar 9, 2023

View reviewed changes

nithinraok previously approved these changes Mar 9, 2023

View reviewed changes

XuesongYang dismissed stale reviews from nithinraok and titu1994 via 033d998 March 10, 2023 04:13

XuesongYang force-pushed the remove-duplicate-codes branch from 4bacce4 to 033d998 Compare March 10, 2023 04:13

XuesongYang requested review from redoctopus, titu1994 and nithinraok March 10, 2023 17:49

redoctopus previously approved these changes Mar 10, 2023

View reviewed changes

rlangman previously approved these changes Mar 10, 2023

View reviewed changes

titu1994 previously requested changes Mar 10, 2023

View reviewed changes

docs/source/asr/examples/kinyarwanda_asr.rst Show resolved Hide resolved

tutorials/asr/asr_adapters/ASR_with_Adapters.ipynb Outdated Show resolved Hide resolved

tutorials/asr/Buffered_Transducer_Inference_with_LCS_Merge.ipynb Outdated Show resolved Hide resolved

XuesongYang added 2 commits March 10, 2023 13:12

unify duplicate definition of manifest read and write func.

a26b68f

Signed-off-by: Xuesong Yang <[email protected]>

extend support of both Path and str.

138ec09

Signed-off-by: Xuesong Yang <[email protected]>

XuesongYang and others added 5 commits March 10, 2023 13:12

update

fab8dd0

update ones in tutorials.

e6d8e8c

Signed-off-by: Xuesong Yang <[email protected]>

[pre-commit.ci] auto fixes from pre-commit.com hooks

41ea897

for more information, see https://pre-commit.ci

add back print statement

6b004e9

Signed-off-by: Xuesong Yang <[email protected]>

Revert "update ones in tutorials."

48af0e5

This reverts commit 033d998. Signed-off-by: Xuesong Yang <[email protected]>

XuesongYang dismissed stale reviews from rlangman and redoctopus via df1899b March 10, 2023 21:13

XuesongYang force-pushed the remove-duplicate-codes branch 2 times, most recently from df1899b to 233322c Compare March 10, 2023 21:16

fix tutorials only as needs.

8a2c9d2

Signed-off-by: Xuesong Yang <[email protected]>

XuesongYang force-pushed the remove-duplicate-codes branch from 233322c to 8a2c9d2 Compare March 10, 2023 21:18

XuesongYang requested review from titu1994, redoctopus and rlangman March 10, 2023 21:19

redoctopus approved these changes Mar 10, 2023

View reviewed changes

XuesongYang merged commit 0ea115b into main Mar 11, 2023

XuesongYang deleted the remove-duplicate-codes branch March 11, 2023 03:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remove duplicate definition of manifest read and write func. #6088

remove duplicate definition of manifest read and write func. #6088

XuesongYang commented Feb 23, 2023 •

edited

Loading

racoiaws commented Feb 24, 2023

XuesongYang commented Mar 8, 2023 •

edited

Loading

titu1994 left a comment

redoctopus left a comment

titu1994 left a comment

remove duplicate definition of manifest read and write func. #6088

remove duplicate definition of manifest read and write func. #6088

Conversation

XuesongYang commented Feb 23, 2023 • edited Loading

What does this PR do ?

Changelog

Usage

Before your PR is "Ready for review"

Who can review?

Additional Information

racoiaws commented Feb 24, 2023

XuesongYang commented Mar 8, 2023 • edited Loading

titu1994 left a comment

Choose a reason for hiding this comment

redoctopus left a comment

Choose a reason for hiding this comment

titu1994 left a comment

Choose a reason for hiding this comment

XuesongYang commented Feb 23, 2023 •

edited

Loading

XuesongYang commented Mar 8, 2023 •

edited

Loading