KEMBAR78
Improve Flake Rules by krishnakalyan3 · Pull Request #1214 · pytorch/audio · GitHub
Skip to content

Conversation

@krishnakalyan3
Copy link
Contributor

Followup of #1172

@krishnakalyan3
Copy link
Contributor Author

Import files that need to be replaced by all

test/torchaudio_unittest/common_utils/__init__.py
torchaudio/backend/__init__.py
torchaudio/functional/__init__.py
torchaudio/sox_effects/__init__.py
torchaudio/models/__init__.py
torchaudio/__init__.py

@krishnakalyan3
Copy link
Contributor Author

krishnakalyan3 commented Jan 31, 2021

@mthrok we have two options according to

from .my_class import MyClass

__all__ = ['MyClass',]

or

from .my_class import MyClass  # noqa: F401

Confirming that the first option was preferred right?

@mthrok
Copy link
Contributor

mthrok commented Feb 1, 2021

@mthrok we have two options according to

from .my_class import MyClass

__all__ = ['MyClass',]

or

from .my_class import MyClass  # noqa: F401

Confirming that the first option was preferred right?

Hi @krishnakalyan3

Thanks for the follow up. Yes, the __all__ is the preferred approach for public APIs, but for internal APIs, noqa is preferred. So, for backend module, let's use noqa.

Copy link
Contributor

@mthrok mthrok left a comment

Choose a reason for hiding this comment

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

@krishnakalyan3 Thank you for working on this. It looks good. Is this ready to merge? (the failing I/O tests are flaky ones that I am hunting down the condition, so we can ignore.)

@krishnakalyan3
Copy link
Contributor Author

krishnakalyan3 commented Feb 8, 2021

@mthrok yes.

@mthrok mthrok merged commit 0f23e6d into pytorch:master Feb 8, 2021
@mthrok
Copy link
Contributor

mthrok commented Feb 8, 2021

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants