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.

Sign up for GitHub

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

Jump to bottom

Enforce import tests on the three domains #3702

Merged
merged 21 commits into from
Feb 18, 2022
Merged

Enforce import tests on the three domains #3702

merged 21 commits into from
Feb 18, 2022

Conversation

titu1994
Copy link
Collaborator

@titu1994 titu1994 commented Feb 18, 2022 •
edited
Loading

What does this PR do ?

Enforce that all top level domain imports must be safe - and no optional dependency should break the base import.

Collection: [ASR, NLP, TTS]

Changelog

  • Add an explicit import test to the three domains, independently of the others.
  • Follow the normal user style of installing nemo - but with individual domains.
  • Error out if any domain breaks it, preventing merge of the PR.

Usage

  • Part of CI test run on Github Actions

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

@titu1994
Add check import test
0bf6833 
Signed-off-by: smajumdar <[email protected]>
@titu1994
Add check import test
927db94 
Signed-off-by: smajumdar <[email protected]>
@titu1994
Add check import test
9acdcc4 
Signed-off-by: smajumdar <[email protected]>
@titu1994
Add check import test
ed02e8f 
Signed-off-by: smajumdar <[email protected]>
@titu1994
Add check import test
42500bf 
Signed-off-by: smajumdar <[email protected]>
@titu1994
Add check import test
432fb8f 
Signed-off-by: smajumdar <[email protected]>
@titu1994
Add check import test
bb4be08 
Signed-off-by: smajumdar <[email protected]>
@titu1994
Add check import test
10e722f 
Signed-off-by: smajumdar <[email protected]>
@titu1994
Add check import test
701615a 
Signed-off-by: smajumdar <[email protected]>
@titu1994
Add check import test
7c6a3a2 
Signed-off-by: smajumdar <[email protected]>
@titu1994
Add check import test
cf138b4 
Signed-off-by: smajumdar <[email protected]>
@titu1994
Add check import test
58c159d 
Signed-off-by: smajumdar <[email protected]>
@titu1994
Format check imports
b709e68 
Signed-off-by: smajumdar <[email protected]>
@titu1994
Remove duplicate test
031bc3f 
Signed-off-by: smajumdar <[email protected]>
ericharper
ericharper approved these changes Feb 18, 2022
View reviewed changes
Copy link
Collaborator

@ericharper ericharper left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@titu1994 titu1994 merged commit fada845 into main Feb 18, 2022
@titu1994 titu1994 deleted the install_checks branch February 18, 2022 05:07
fayejf pushed a commit that referenced this pull request Mar 2, 2022
@titu1994 @ericharper @fayejf
Enforce import tests on the three domains (#3702)
7355909 
* Add check import test

Signed-off-by: smajumdar <[email protected]>

* Add check import test

Signed-off-by: smajumdar <[email protected]>

* Add check import test

Signed-off-by: smajumdar <[email protected]>

* Add check import test

Signed-off-by: smajumdar <[email protected]>

* Add check import test

Signed-off-by: smajumdar <[email protected]>

* Add check import test

Signed-off-by: smajumdar <[email protected]>

* Add check import test

Signed-off-by: smajumdar <[email protected]>

* Add check import test

Signed-off-by: smajumdar <[email protected]>

* Add check import test

Signed-off-by: smajumdar <[email protected]>

* Add check import test

Signed-off-by: smajumdar <[email protected]>

* Add check import test

Signed-off-by: smajumdar <[email protected]>

* Add check import test

Signed-off-by: smajumdar <[email protected]>

* Format check imports

Signed-off-by: smajumdar <[email protected]>

* Remove duplicate test

Signed-off-by: smajumdar <[email protected]>

* apex arg guard

Signed-off-by: ericharper <[email protected]>

* apex arg guard

Signed-off-by: ericharper <[email protected]>

* apex arg guard

Signed-off-by: ericharper <[email protected]>

* add guard for bert

Signed-off-by: ericharper <[email protected]>

* build_position_ids

Signed-off-by: ericharper <[email protected]>

Co-authored-by: Eric Harper <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericharper ericharper ericharper approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@titu1994 @ericharper