Couple comments on reading

[danpovey]

https://github.com/pzelasko/lhotse/blob/7555df605def57836c9454ae44aac95c504d86b0/lhotse/audio.py#L21

There should possibly be at least a TODO here to support getting a subset of channels. I'm thinking of scenarios like Switchboard. Since I think typical storage formats store the different channels interleaved, it may be just as efficient to read all of them and then select the needed ones. But there are scenarios where different channels are stored separately (AMI). We have to figure out at what level to do this.

Regarding the raw-PCM thing: I think if there is any common/universal format it would probably be wav, not raw pcm, as raw pcm is too easy to get wrong (no error if wrong format). I didn't really understand the AudioSource code well as your Python style is very modern... e.g. no init function.

[pzelasko]

Regarding subests of channels - it's already supported, note the test_get_audio_multichannel - I'll document it better. I elaborated more in issue #4

Regarding the raw-PCM thing: I think if there is any common/universal format it would probably be wav, not raw pcm, as raw pcm is too easy to get wrong (no error if wrong format).

That's just for the command type of input, where we could always place sox at the end of the pipe to convert practically anything to PCM, and it is very easy to load with numpy as it's just a cast of binary buffer to int16. But we can make it work with wav too I guess.

I didn't really understand the AudioSource code well as your Python style is very modern... e.g. no init function.

Alright, let me elaborate on that. Python 3.7 introduced dataclass, based on attrs library. It basically reduces the amount of boilerplate code you need to write. You can specify the members similarly as in C++, e.g.:

@dataclass
class Foo:
  x: int
  y: List[str]

and it automatically creates default __init__, __eq__, __repr__, and can also create comparison operators, __hash__, or impose immutability with extra arguments to dataclass. The __init__ would look something like:

def __init__(self, x: int, y: List[str]):
  self.x = x
  self.y = y
  self.__post_init__()

It also provides a method asdict which can convert a dataclass into a nice dictionary, making serialization very easy (notice the code is quite terse for what it does). There's a __post_init__ that's automatically called in the auto-generated constructor, it allows you to customize __init__ without having to write the boilerplate.

Also, I find the typing module extremely useful for static code analysis and improving documentation. List[int] type means the variable will be a list of int, Optional means it can be None, Union[int, str] means it can be either an int or str.

[csukuangfj]

Alright, let me elaborate on that. Python 3.7 introduced dataclass

The users of this library are forced to use Python 3.7. If some of them do not have
sudo permission, they will have difficulties upgrading their python version to 3.7

[danpovey]

Mm. My feeling is the python3.7 thing may be OK... this is a rather forward-looking project, and my feeling is enough people have python3.7 (or could install it in user-space).
If it's a problem later we can always change the code, but for now I don't want to slow this project down by worrying too much about such thigns.

[pzelasko]

The users can also install Anaconda/Miniconda without sudo permissions and use any python version they want. czw., 7 maj 2020, 23:16 użytkownik Daniel Povey <notifications@github.com> napisał:

…

Mm. My feeling is the python3.7 thing may be OK... this is a rather forward-looking project, and my feeling is enough people have python3.7 (or could install it in user-space). If it's a problem later we can always change the code, but for now I don't want to slow this project down by worrying too much about such thigns. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#2 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADZRKQCXOVNX7E2FT2E6L5LRQN2PZANCNFSM4MRDBQ5A> .

[jtrmal]

plus python 3.7 was released mid 2018... IMO it's ok y.

…

On Fri, May 8, 2020 at 8:25 AM Piotr Żelasko ***@***.***> wrote: The users can also install Anaconda/Miniconda without sudo permissions and use any python version they want. czw., 7 maj 2020, 23:16 użytkownik Daniel Povey ***@***.***> napisał: > Mm. My feeling is the python3.7 thing may be OK... this is a rather > forward-looking project, and my feeling is enough people have python3.7 (or > could install it in user-space). > If it's a problem later we can always change the code, but for now I don't > want to slow this project down by worrying too much about such thigns. > > — > You are receiving this because you modified the open/close state. > Reply to this email directly, view it on GitHub > <#2 (comment)>, or > unsubscribe > < https://github.com/notifications/unsubscribe-auth/ADZRKQCXOVNX7E2FT2E6L5LRQN2PZANCNFSM4MRDBQ5A > > . > — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACUKYX23ZHEYPS7ZYCABFXTRQP22TANCNFSM4MRDBQ5A> .