Apply new Black style changes #690
Conversation
.pre-commit-config.yaml
Outdated
| hooks: | ||
| - id: black | ||
| args: [--line-length=80] | ||
| args: ["--line-length=88"] | ||
| additional_dependencies: ['click==8.0.1'] |
There was a problem hiding this comment.
shouldn't it use click 8.1.0 like in CI?
There was a problem hiding this comment.
Good catch. I'll change it!
Yeah apparently I removed about 8k lines of code. |
Just remember to never do this if you're being paid per LOC ;) |
|
Is it ready to merge |
Yes! |
| flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics | ||
| # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide | ||
| flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 \ | ||
| --statistics --extend-ignore=E203,E266,E501,F401,E402,F403,F841,W503 |
There was a problem hiding this comment.
@pzelasko @csukuangfj
There's just this one annoying little detail. I had to add E501 to the flake8 check, but it actually ignores line length issues. This is because there are about a few hundred violations in the repo that could not be fixed by black because they are in the comments or docstrings.
|
Shall I use "squash merge" or just "merge"? |
Piotr advised that you should merge "without" squashing so that all the commit hashes are retained. |
Thanks |
|
Why have some of the files gone blank? Was this intended? Some of the scripts are not working after this |
No, that is not intended. |
| ) | ||
|
|
||
| parser.add_argument( | ||
| "--tokens-file", | ||
| type=str, | ||
| help="Path to tokens.txt" "Used only when method is ctc-decoding", | ||
| help="Path to tokens.txtUsed only when method is ctc-decoding", |
There was a problem hiding this comment.
There should be a space between ".txt" and "Used".
Looks like black cannot handle such a case properly.
There was a problem hiding this comment.
I think this and the other issues were caused due to using the --experimental-string-processing feature which is not stable yet. I will run black again without this flag to fix the issues
| @@ -1,818 +0,0 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
Looks like this file is removed.
| @@ -1,388 +0,0 @@ | |||
| #!/usr/bin/env python3 | |||
| @@ -1,1157 +0,0 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
Looks like many files from this directory are removed.
| @@ -25,15 +25,10 @@ | |||
| from typing import List, Tuple | |||
|
|
|||
| import torch | |||
| from lhotse import ( | |||
| from lhotse import ( # fmt: off; See the following for why LilcomChunkyWriter is preferred; https://github.com/k2-fsa/icefall/pull/404; https://github.com/lhotse-speech/lhotse/pull/527; fmt: on | |||
There was a problem hiding this comment.
This comment looks a bit messed up also
| f'Unsupported direction: {direction}. " \ | ||
| "Expect either "left" or "right"' | ||
| f'Unsupported direction: {direction}. " "Expect either "left"' | ||
| ' or "right"' |
There was a problem hiding this comment.
It looks like this string was wrongly mixing single quotes and double quotes, and was formatted weirdly
|
@csukuangfj please revert the merge for now. I'll take a look at this today and fix the issues. |
Is it possible to just upload the missing files without reverting? |
Yeah that can be done too, but I'm concerned that the experiment string processing may have introduced additional problems which we haven't caught yet. I shouldn't have used this unstable feature in the first place. |
|
ok, I will revert it then. |
Thanks. |
This PR applies the new line-length configuration (see lhotse-speech/lhotse#890) on all the files.
We have added a
.git-blame-ignore-revsfile containing the hash of the main commit to ensure that git blame history is not affected. Please follow the instructions in this link to enable this setting in your git config. This option is available in git 2.23 and later (and also enabled by default on Github).