Prevent accidental renaming when using with_suffix #1192
Conversation
lhotse/features/io.py
Outdated
| @@ -280,7 +280,8 @@ def write(self, key: str, value: np.ndarray) -> str: | |||
| # too many files in a single directory. | |||
| subdir = self.storage_path_ / key[:3] | |||
| subdir.mkdir(exist_ok=True) | |||
| output_features_path = (subdir / key).with_suffix(".llc") | |||
| p = subdir / key | |||
| output_features_path = p.with_suffix(p.suffix + ".llc" if p.suffix != ".llc" else ".llc") | |||
There was a problem hiding this comment.
Is the if/else needed here? (Same for all below changes)
There was a problem hiding this comment.
The if/else is to prevent adding of suffix if p contains the suffix for some reason say "test.llc". So it will not become "test.llc.llc".
Which is the expected behavior mentioned here
https://github.com/lhotse-speech/lhotse/blob/master/lhotse/features/io.py#L443
|
I am not too sure what sort of test is needed, is it those similar to the one in the main testing folder like this: |
|
A test similar to that one would do. The goal is to cover a scenario in which the test would have failed before the change, but passes after it. You might want to use either
That test is correct, |
|
Thanks for the clarification. Have added in the test. Tested it on my side, should be working. |
Head branch was pushed to by a user without write access
Head branch was pushed to by a user without write access
|
Gosh.. so sorry.. had trouble using the isort on my vscode.. and missed one of the changes. |
Fixes #1183
The use of
.with_suffixreplaces the strings after the., which leads to issues when the key/path contains.in the name.