-
Notifications
You must be signed in to change notification settings - Fork 217
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.
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
WIP kaldi import/export refactoring proof of concept/design #693
base: master
Are you sure you want to change the base?
WIP kaldi import/export refactoring proof of concept/design #693
Conversation
…d anything else than the original interface
seems like the test fail is because of something wrong with wpe |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for commenting on it so late; first I postponed it and then I forgot about it for a while.
Why do you want to refactor this? I'm not opposed to it but as-is I don't see the benefit. Are you intending to add more types of input / output formatters? What would they support?
import contextlib | ||
|
||
|
||
class KaldiFormatterBase: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like both input and output formatters are context managers and don't have a common interface otherwise; not sure we need a common base class for that.
@@ -50,6 +290,22 @@ def get_duration( | |||
return info.duration | |||
|
|||
|
|||
class KaldiDirectory: | |||
def __init__(self, path: Pathlike, options: dict): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
options as dict is are a little hacky, why not define keyword arguments?
I plan to get back to this once I have a little bit more bandwidth available |
Import/export from Kaldi seems to be a popular feature. Perhaps we should try to get these issue resolved soon. |
let me try to revive this today... I feel like I might have different
opinion on how I did stuff after I had it on backburner for a few months
But feel free to comment on this if you feel you have a better idea(s)
y.
…On Wed, Sep 28, 2022 at 9:33 AM Desh Raj ***@***.***> wrote:
Import/export from Kaldi seems to be a popular feature. Perhaps we should
try to get these issue resolved soon.
—
Reply to this email directly, view it on GitHub
<#693 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACUKYXZXR7A47MUYM5YVG5DWARCK5ANCNFSM5U4PMLBA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
This is just an attempt at refactoring the code into a few classes to enable override/specify input or output format for specifically formatted corpora. For now, no customization is enabled, it's just the refactoring itself to discuss the design.
Passes the kaldi tests, tho