Skip to content

Conversation

@msaroufim
Copy link
Member

No description provided.

@msaroufim
Copy link
Member Author

msaroufim commented Apr 23, 2021

@mthrok this passes the flake8 issues. Had to create a new PR since my updated commits weren't being reflected in the old PR

May be helpful to open up a separate task for flake8 standardization since many files don't pass the linter right now with the default flake8 settings

The github code review UI is weird and skips the new line at the end of the file, if you click edit on the file you'll see that the empty line is indeed there

Tests look good overall but still some flaky ones, are there any low hanging fruit to make them less flaky?

@mthrok
Copy link
Contributor

mthrok commented Apr 23, 2021

Hi @msaroufim

The PR looks good. We just fixed Windows issue in #1474, can you incorporate the change?

Regarding the linting, indeed the torchaudio's configuration is different from the default one, but that's the way repo was set up, so I think it's better kept that way. Also the change about the linting always include certain degree of subjectivity, which will allow anyone to object from any angle, and often times, which leads the conversation nowhere, so I think it's better not to touch it. Even applying black is not straightforward because the publicly available version and fb internal version are different, and later internal tool will try to format differently.
As long as flake8 passes, the style is somewhat in a good side.

@msaroufim msaroufim closed this Apr 26, 2021
mthrok pushed a commit to mthrok/audio that referenced this pull request Dec 13, 2022
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