Skip to content
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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jtrmal
Copy link
Collaborator

@jtrmal jtrmal commented May 2, 2022

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

@jtrmal
Copy link
Collaborator Author

jtrmal commented May 2, 2022

seems like the test fail is because of something wrong with wpe

Copy link
Collaborator

@pzelasko pzelasko left a 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:
Copy link
Collaborator

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):
Copy link
Collaborator

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?

@jtrmal
Copy link
Collaborator Author

jtrmal commented May 18, 2022

I plan to get back to this once I have a little bit more bandwidth available

@desh2608
Copy link
Collaborator

Import/export from Kaldi seems to be a popular feature. Perhaps we should try to get these issue resolved soon.

@jtrmal
Copy link
Collaborator Author

jtrmal commented Sep 28, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants