Normalize output path names for recipes

[desh2608]

This PR "normalizes" the output manifests for the recipes:

• All JSON type outputs are changed to JSONL (ses this comment)
• Manifests are named as <corpus-name>_<manifest-type>_<part>.jsonl, e.g., "ami_recordings_dev.jsonl". This is to ensure that manifests from multiple corpora can be saved to same directory without name-based confusion as would occur with names like "recordings_dev.jsonl".

Obviously, we would have to modify the data preparation in icefall to be compatible with these changes, but I feel it's better to switch to this normalized naming scheme now rather than when it would break a lot more recipes.

[desh2608]

BTW about 1/4th of the recipes already follow this naming (e.g. gigaspeech, spgispeech, swbd, ...)

[pzelasko]

A big +1 from me, thank you for this effort 🙏🏻

Some suggestions:

• we could adopt a convention that corpora without splits would be named <corpus-name>_<manifest-type>_all.jsonl.
• when a corpus specifies multiple types of sub-splits (e.g. mtedx has language) put these extra parts before <manifest-type> so that if we do parts = path.stem.split("_") then the following two are always true: parts[-1] == '<train/dev/test/all-split>', and parts[-2] == <manifest-type>, maybe that can help some automation if people combine more data together.

If you agree, could you add these tips to https://github.com/lhotse-speech/lhotse/blob/master/docs/corpus.rst#adding-new-corpora ?

[jtrmal]

if this, then why not just having them gzipped by default? some of them are and some of them aren't y.

…

On Mon, May 16, 2022 at 1:39 PM Piotr Żelasko ***@***.***> wrote: A big +1 from me, thank you for this effort 🙏🏻 Some suggestions: - we could adopt a convention that corpora without splits would be named <corpus-name>_<manifest-type>_all.jsonl. - when a corpus specifies multiple types of sub-splits (e.g. mtedx has language) put these extra parts before <manifest-type> so that if we do parts = path.stem.split("_") then the following two are always true: parts[-1] == '<train/dev/test/all-split>', and parts[-2] == <manifest-type>, maybe that can help some automation if people combine more data together. — Reply to this email directly, view it on GitHub <#712 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACUKYX6ILAMHISQ4L2KXXWDVKKB3JANCNFSM5WCG3EAQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[desh2608]

@pzelasko @jtrmal both of those suggestions make sense to me. I didn't add any gzipping since my understanding was we mainly do it for large(ish) manifests, but I can add that too if it's the consensus.

[jtrmal]

I defer to Piotr's opinion :) y.

…

On Mon, May 16, 2022 at 1:56 PM Desh Raj ***@***.***> wrote: @pzelasko <https://github.com/pzelasko> @jtrmal <https://github.com/jtrmal> both of those suggestions make sense to me. I didn't add any gzipping since my understanding was we mainly do it for large(ish) manifests, but I can add that too if it's the consensus. — Reply to this email directly, view it on GitHub <#712 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACUKYX2EPUN4NMI7CCSNOM3VKKD3XANCNFSM5WCG3EAQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[pzelasko]

I don't see anything to lose and I guess it makes people's lives easier.

[desh2608]

Done :)

[pzelasko]

Awesome!

[ngoel17]

@desh2608 - Just adding my comment that I sent to Piotr. I did this several weeks ago - and the motivation was that I wanted to use recipes/utils on kaldi imported data.

I found that if I do the following modifications to the recipes/utils.py my life becomes super easy. But this will not work with the lhotse recipes because the naming convention for kaldi imports is different than the convention for lhotse recipes.

How about making both the same? i.e. dataset_name/supervisons.jsonl.gz etc. Please look.

Here is a diff -

:param prefix: Optional common prefix for the manifest files (underscore is automatically added).
@@ -39,8 +42,6 @@ def read_manifests_if_cached(
     """
     if output_dir is None:
         return {}
-    if prefix and not prefix.endswith("_"):
-        prefix = f"{prefix}_"
     if suffix.startswith("."):
         suffix = suffix[1:]
     if lazy and not suffix.startswith("jsonl"):
@@ -48,17 +49,19 @@ def read_manifests_if_cached(
             f"Only JSONL manifests can be opened lazily (got suffix: '{suffix}')"
         )
     manifests = defaultdict(dict)
-    for part in dataset_parts:
-        for manifest in types:
-            path = output_dir / f"{prefix}{manifest}_{part}.{suffix}"
-            if not path.is_file():
-                continue
-            if lazy:
-                manifests[part][manifest] = TYPES_TO_CLASSES[manifest].from_jsonl_lazy(
-                    path
-                )
-            else:
-                manifests[part][manifest] = load_manifest(path)
+    for dataset in datasets:
+        for part in dataset_parts:
+            for manifest in types:
+                path = output_dir / f"./{prefix}/{dataset}/{manifest}{part}.{suffix}"
+                if not path.is_file():
+                    [logging.info](http://logging.info/)(f'{path} not found')
+                    continue
+                if lazy:
+                    manifests[dataset][manifest] = TYPES_TO_CLASSES[manifest].from_jsonl_lazy(
+                        path
+                    )
+                else:
+                    manifests[dataset][manifest] = load_manifest(path)
     return dict(manifests)