From 318aaa8b84ad3a212ccb63479c39eef92238d52c Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Mon, 30 Dec 2019 20:09:18 -0800 Subject: [PATCH 01/15] MP3Loader: add the "audio/mpeg" mime type --- ultratrace2/model/files/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ultratrace2/model/files/__init__.py b/ultratrace2/model/files/__init__.py index bf613f7..62d754b 100644 --- a/ultratrace2/model/files/__init__.py +++ b/ultratrace2/model/files/__init__.py @@ -37,7 +37,9 @@ from .loaders import MP3Loader __register( - [".mp3"], ["audio/mp3", "audio/MPA", "audio/mpa-robust"], MP3Loader, + [".mp3"], + ["audio/mp3", "audio/mpeg", "audio/MPA", "audio/mpa-robust"], + MP3Loader, ) except ImportError as e: logger.warning(e) From df6f04761930435076c33d54f8eb34042d2d8267 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Mon, 30 Dec 2019 20:10:06 -0800 Subject: [PATCH 02/15] FileBundle+FileLoaderBase: Add __eq__ method This will simplify testing :^) --- ultratrace2/model/files/bundle.py | 8 ++++++++ ultratrace2/model/files/loaders/base.py | 3 +++ 2 files changed, 11 insertions(+) diff --git a/ultratrace2/model/files/bundle.py b/ultratrace2/model/files/bundle.py index 7bf73ae..e4b5b34 100644 --- a/ultratrace2/model/files/bundle.py +++ b/ultratrace2/model/files/bundle.py @@ -53,6 +53,14 @@ def set_sound_file(self, sound_file: SoundFileLoader) -> None: def __repr__(self): return f'Bundle("{self.name}",{self.alignment_file},{self.image_set_file},{self.sound_file})' + def __eq__(self, other): + return ( + self.name == other.name + and self.alignment_file == other.alignment_file + and self.image_set_file == other.image_set_file + and self.sound_file == other.sound_file + ) + class FileBundleList: diff --git a/ultratrace2/model/files/loaders/base.py b/ultratrace2/model/files/loaders/base.py index 99b4b6e..77b62d3 100644 --- a/ultratrace2/model/files/loaders/base.py +++ b/ultratrace2/model/files/loaders/base.py @@ -36,6 +36,9 @@ def from_file(cls: Type[Self], path: str) -> Self: NB: If this concrete method fails to load the data at the given path, then it should throw a `FileLoadError`.""" + def __eq__(self, other): + return self.get_path() == other.get_path() and type(self) == type(other) + class IntervalBase(Protocol): def get_start(self) -> float: From be65022ac0ea19d8b1bb20f94867654dd88f3c97 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Mon, 30 Dec 2019 20:10:59 -0800 Subject: [PATCH 03/15] FileBundleList: Fix bug where we were passing path instead of filename --- ultratrace2/model/files/bundle.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ultratrace2/model/files/bundle.py b/ultratrace2/model/files/bundle.py index e4b5b34..2a6c705 100644 --- a/ultratrace2/model/files/bundle.py +++ b/ultratrace2/model/files/bundle.py @@ -123,7 +123,7 @@ def build_from_dir( continue if name not in bundles: - bundles[name] = FileBundle(filepath) + bundles[name] = FileBundle(name) try: loaded_file = file_loader.from_file(filepath) From 94a2e99c1d6c8baa47ff8062579f576628d9a943 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Mon, 30 Dec 2019 20:12:01 -0800 Subject: [PATCH 04/15] FileLoaderBase+registry: Add priority to loaders Priority is per-(direct subclass of)FileLoaderBase -- i.e., each concrete subclass of, say, SoundFileLoader must have a unique priority. See #124 for more context :^) --- ultratrace2/model/files/loaders/base.py | 4 ++ ultratrace2/model/files/loaders/flac.py | 4 ++ ultratrace2/model/files/loaders/mp3.py | 4 ++ ultratrace2/model/files/loaders/ogg.py | 4 ++ ultratrace2/model/files/loaders/wav.py | 4 ++ ultratrace2/model/files/registry.py | 62 +++++++++++++++++-------- 6 files changed, 62 insertions(+), 20 deletions(-) diff --git a/ultratrace2/model/files/loaders/base.py b/ultratrace2/model/files/loaders/base.py index 77b62d3..9822f8a 100644 --- a/ultratrace2/model/files/loaders/base.py +++ b/ultratrace2/model/files/loaders/base.py @@ -39,6 +39,10 @@ def from_file(cls: Type[Self], path: str) -> Self: def __eq__(self, other): return self.get_path() == other.get_path() and type(self) == type(other) + @staticmethod + def get_priority() -> int: + return 0 + class IntervalBase(Protocol): def get_start(self) -> float: diff --git a/ultratrace2/model/files/loaders/flac.py b/ultratrace2/model/files/loaders/flac.py index 6f59c4c..d307ac9 100644 --- a/ultratrace2/model/files/loaders/flac.py +++ b/ultratrace2/model/files/loaders/flac.py @@ -26,3 +26,7 @@ def from_file(cls, path: str) -> "FLACLoader": raise FileLoadError( f"Invalid FLAC ({path}), unable to read: {str(e)}" ) from e + + @staticmethod + def get_priority() -> int: + return 4 diff --git a/ultratrace2/model/files/loaders/mp3.py b/ultratrace2/model/files/loaders/mp3.py index 75595b3..3187dd2 100644 --- a/ultratrace2/model/files/loaders/mp3.py +++ b/ultratrace2/model/files/loaders/mp3.py @@ -26,3 +26,7 @@ def from_file(cls, path: str) -> "MP3Loader": raise FileLoadError( f"Invalid MP3 ({path}), unable to read: {str(e)}" ) from e + + @staticmethod + def get_priority() -> int: + return 1 diff --git a/ultratrace2/model/files/loaders/ogg.py b/ultratrace2/model/files/loaders/ogg.py index ebc332f..15d57ac 100644 --- a/ultratrace2/model/files/loaders/ogg.py +++ b/ultratrace2/model/files/loaders/ogg.py @@ -26,3 +26,7 @@ def from_file(cls, path: str) -> "OggLoader": raise FileLoadError( f"Invalid Ogg ({path}), unable to read: {str(e)}" ) from e + + @staticmethod + def get_priority() -> int: + return 2 diff --git a/ultratrace2/model/files/loaders/wav.py b/ultratrace2/model/files/loaders/wav.py index cf931ce..74ea52a 100644 --- a/ultratrace2/model/files/loaders/wav.py +++ b/ultratrace2/model/files/loaders/wav.py @@ -26,3 +26,7 @@ def from_file(cls, path: str) -> "WAVLoader": raise FileLoadError( f"Invalid WAV ({path}), unable to read: {str(e)}" ) from e + + @staticmethod + def get_priority() -> int: + return 3 diff --git a/ultratrace2/model/files/registry.py b/ultratrace2/model/files/registry.py index 4766fb3..c71f699 100644 --- a/ultratrace2/model/files/registry.py +++ b/ultratrace2/model/files/registry.py @@ -1,19 +1,28 @@ import mimetypes import os -from collections import defaultdict -from typing import DefaultDict, Optional, Sequence, Set, Type +from typing import Dict, Mapping, Optional, Sequence, Set, Type, Union + +from .loaders.base import ( + FileLoaderBase, + AlignmentFileLoader, + ImageSetFileLoader, + SoundFileLoader, +) -from .loaders.base import FileLoaderBase +AbstractLoader = Union[ + Type[AlignmentFileLoader], Type[ImageSetFileLoader], Type[SoundFileLoader] +] # global maps -__extension_to_loaders_map: DefaultDict[str, Set[Type[FileLoaderBase]]] = defaultdict( - set -) -__mime_type_to_loaders_map: DefaultDict[str, Set[Type[FileLoaderBase]]] = defaultdict( - set -) +__extension_to_loaders_map: Dict[str, Type[FileLoaderBase]] = {} +__mime_type_to_loaders_map: Dict[str, Type[FileLoaderBase]] = {} +__loader_priorities_map: Mapping[AbstractLoader, Set[int]] = { + AlignmentFileLoader: set(), + ImageSetFileLoader: set(), + SoundFileLoader: set(), +} def register_loader_for_extensions_and_mime_types( @@ -30,11 +39,26 @@ def register_loader_for_extensions_and_mime_types( loader_cls: A file loader which knows how to load files with the given file extensions and MIME types """ + loader_type: AbstractLoader + if issubclass(loader_cls, AlignmentFileLoader): + loader_type = AlignmentFileLoader + elif issubclass(loader_cls, ImageSetFileLoader): + loader_type = ImageSetFileLoader + elif issubclass(loader_cls, SoundFileLoader): + loader_type = SoundFileLoader + else: + raise ValueError(f"Invalid loader class: {loader_cls.__name__}") + priority = loader_cls.get_priority() + if priority in __loader_priorities_map[loader_type]: + raise ValueError( + f"Cannot have duplicate priorities for loader type {loader_type.__name__}" + ) + for extension in extensions: - __extension_to_loaders_map[extension].add(loader_cls) + __extension_to_loaders_map[extension] = loader_cls for mime_type in mime_types: - __mime_type_to_loaders_map[mime_type].add(loader_cls) + __mime_type_to_loaders_map[mime_type] = loader_cls def get_loader_for(path: str) -> Optional[Type[FileLoaderBase]]: @@ -45,17 +69,15 @@ def get_loader_for(path: str) -> Optional[Type[FileLoaderBase]]: # Early return since we can't possibly match anymore return None - loader_clses_by_extension = __extension_to_loaders_map[extension] - loader_clses_by_mime_type = __mime_type_to_loaders_map[mime_type] + loader_cls_by_extension = __extension_to_loaders_map[extension] + loader_cls_by_mime_type = __mime_type_to_loaders_map[mime_type] # NB: Use set-intersection (could potentially use set-union instead). - loader_clses = loader_clses_by_extension & loader_clses_by_mime_type - - if len(loader_clses) == 0: + if loader_cls_by_extension is None or loader_cls_by_mime_type is None: return None - elif len(loader_clses) == 1: - return loader_clses.pop() + if loader_cls_by_extension == loader_cls_by_mime_type: + return loader_cls_by_extension else: - raise NotImplementedError( - f"Found multiple Loaders for path '{path}': {','.join(map(str, loader_clses))}" + raise ValueError( + f"Warning: got {loader_cls_by_extension.__name__} for {extension} and {loader_cls_by_mime_type} for {mime_type}" ) From 1786f5e5b31439d7c9d569ade3aff90094dd4b9f Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Mon, 30 Dec 2019 20:16:01 -0800 Subject: [PATCH 05/15] registry: Switch to using python-magic to detect mime types Turns out that the builtin `mimetypes` module can't detect using anything except file extension, so that's basically useless for us. --- requirements.txt | 1 + ultratrace2/model/files/registry.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/requirements.txt b/requirements.txt index 8bc75c2..304549e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -14,3 +14,4 @@ TextGrid==1.5 tqdm==4.40.2 ttkthemes==2.4.0 xparser==0.0.4 +python-magic==0.4.15 diff --git a/ultratrace2/model/files/registry.py b/ultratrace2/model/files/registry.py index c71f699..bd17898 100644 --- a/ultratrace2/model/files/registry.py +++ b/ultratrace2/model/files/registry.py @@ -1,4 +1,4 @@ -import mimetypes +import magic # type: ignore import os from typing import Dict, Mapping, Optional, Sequence, Set, Type, Union @@ -64,7 +64,7 @@ def register_loader_for_extensions_and_mime_types( def get_loader_for(path: str) -> Optional[Type[FileLoaderBase]]: _, extension = os.path.splitext(path.lower()) - mime_type, _ = mimetypes.guess_type(path) + mime_type = magic.Magic(mime=True).from_file(path) if mime_type is None: # Early return since we can't possibly match anymore return None From 9ba305d60d17a766871087beaa13dbd85368b869 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Mon, 30 Dec 2019 20:16:22 -0800 Subject: [PATCH 06/15] FileBundle: Add more tests ... not all of these pass at the moment! --- ultratrace2/model/files/tests/test_bundle.py | 145 ++++++++++++++++++- 1 file changed, 142 insertions(+), 3 deletions(-) diff --git a/ultratrace2/model/files/tests/test_bundle.py b/ultratrace2/model/files/tests/test_bundle.py index fb0e558..fb6689f 100644 --- a/ultratrace2/model/files/tests/test_bundle.py +++ b/ultratrace2/model/files/tests/test_bundle.py @@ -1,6 +1,9 @@ +import os import pytest from ..bundle import FileBundle, FileBundleList +from ..loaders import DICOMLoader, MP3Loader, TextGridLoader, WAVLoader +from ..loaders.base import AlignmentFileLoader, ImageSetFileLoader, SoundFileLoader @pytest.mark.parametrize( @@ -32,11 +35,147 @@ def test_build_from_nonexistent_dir(mocker) -> None: @pytest.mark.parametrize( - "source_dir,bundle_map", [("./test-data/example-bundles/ex000", {})], + "source_dir,expected_file_map,should_emit_warning", + [ + ( + "./test-data/example-bundles/ex000", + {"file00": [(TextGridLoader, "file00.TextGrid")]}, + False, + ), + ( + "./test-data/example-bundles/ex001", + { + "file00": [(TextGridLoader, "file00.TextGrid")], + "file01": [(TextGridLoader, "file01.TextGrid")], + }, + False, + ), + ( + "./test-data/example-bundles/ex002", + { + "file00": [ + (MP3Loader, "file00.mp3"), + (TextGridLoader, "file00.TextGrid"), + ] + }, + False, + ), + ( + "./test-data/example-bundles/ex003", + { + "file00": [(TextGridLoader, "file00.TextGrid")], + "file01": [(MP3Loader, "file01.mp3")], + }, + False, + ), + ( + "./test-data/example-bundles/ex004", + { + "file00": [ + (DICOMLoader, "file00.dicom"), + (MP3Loader, "file00.mp3"), + (TextGridLoader, "file00.TextGrid"), + ] + }, + False, + ), + ("./test-data/example-bundles/ex005", {}, False), + ("./test-data/example-bundles/ex006", {}, True), + ( + "./test-data/example-bundles/ex007", + {"file00": [(TextGridLoader, "file00.TextGrid")]}, + True, + ), + ( + "./test-data/example-bundles/ex008", + { + "file00": [ + (DICOMLoader, "file00.dicom"), + (MP3Loader, "file00.mp3"), + (TextGridLoader, "file00.TextGrid"), + ], + "file01": [ + (DICOMLoader, "file01.dicom"), + (MP3Loader, "file01.mp3"), + (TextGridLoader, "file01.TextGrid"), + ], + "file02": [ + (DICOMLoader, "file02.dicom"), + (MP3Loader, "file02.mp3"), + (TextGridLoader, "file02.TextGrid"), + ], + }, + False, + ), + ( + "./test-data/example-bundles/ex009", + {"file00": [(WAVLoader, "file00.wav")]}, + True, + ), # FIXME + ( + "./test-data/example-bundles/ex010", + {"file00": [(TextGridLoader, "sub00/file00.TextGrid")]}, + False, + ), + ( + "./test-data/example-bundles/ex011", + { + "file00": [(TextGridLoader, "sub00/file00.TextGrid")], + "file01": [(TextGridLoader, "sub01/sub00/sub00/sub00/file01.TextGrid")], + }, + False, + ), + ( + "./test-data/example-bundles/ex012", + { + "file00": [ + (MP3Loader, "sub01/sub00/sub00/sub00/file00.mp3"), + (TextGridLoader, "sub00/file00.TextGrid"), + ] + }, + False, + ), + ( + "./test-data/example-bundles/ex013", + {"file00": [(TextGridLoader, "sub00/file00.TextGrid")]}, + True, + ), # FIXME + # ("./test-data/example-bundles/ex014", FIXME), + # ("./test-data/example-bundles/ex015", FIXME), + # ("./test-data/example-bundles/ex016", FIXME), + ], ) -def test_build_from_dir(mocker, source_dir, bundle_map) -> None: +def test_build_from_dir( + mocker, source_dir, expected_file_map, should_emit_warning +) -> None: mock_file_bundle_list_constructor = mocker.patch( "ultratrace2.model.files.bundle.FileBundleList.__init__", return_value=None, ) + mock_warning = mocker.patch("ultratrace2.model.files.bundle.logger.warning") FileBundleList.build_from_dir(source_dir) - mock_file_bundle_list_constructor.assert_called_with(bundle_map) + expected_bundles = {} + for expected_name, expected_files in expected_file_map.items(): + alignment_file = None + image_set_file = None + sound_file = None + for loader, source_subpath in expected_files: + source_path = os.path.abspath(os.path.join(source_dir, source_subpath)) + if issubclass(loader, AlignmentFileLoader): + alignment_file = loader.from_file(source_path) + elif issubclass(loader, ImageSetFileLoader): + image_set_file = loader.from_file(source_path) + elif issubclass(loader, SoundFileLoader): + sound_file = loader.from_file(source_path) + else: + raise RuntimeError("malformed input") + expected_bundles[expected_name] = FileBundle( + name=expected_name, + alignment_file=alignment_file, + image_set_file=image_set_file, + sound_file=sound_file, + ) + mock_file_bundle_list_constructor.assert_called_with(expected_bundles) + if should_emit_warning: + mock_warning.assert_called_once() + else: + mock_warning.assert_not_called() From bbe0805831b2557d9c20af2a53b39097d938f249 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Mon, 30 Dec 2019 22:03:22 -0800 Subject: [PATCH 07/15] FileBundle: Fix test failures --- ultratrace2/model/files/registry.py | 4 ++-- ultratrace2/model/files/tests/test_bundle.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ultratrace2/model/files/registry.py b/ultratrace2/model/files/registry.py index bd17898..34fc9df 100644 --- a/ultratrace2/model/files/registry.py +++ b/ultratrace2/model/files/registry.py @@ -69,8 +69,8 @@ def get_loader_for(path: str) -> Optional[Type[FileLoaderBase]]: # Early return since we can't possibly match anymore return None - loader_cls_by_extension = __extension_to_loaders_map[extension] - loader_cls_by_mime_type = __mime_type_to_loaders_map[mime_type] + loader_cls_by_extension = __extension_to_loaders_map.get(extension, None) + loader_cls_by_mime_type = __mime_type_to_loaders_map.get(mime_type, None) # NB: Use set-intersection (could potentially use set-union instead). if loader_cls_by_extension is None or loader_cls_by_mime_type is None: diff --git a/ultratrace2/model/files/tests/test_bundle.py b/ultratrace2/model/files/tests/test_bundle.py index fb6689f..7fef02a 100644 --- a/ultratrace2/model/files/tests/test_bundle.py +++ b/ultratrace2/model/files/tests/test_bundle.py @@ -111,7 +111,7 @@ def test_build_from_nonexistent_dir(mocker) -> None: "./test-data/example-bundles/ex009", {"file00": [(WAVLoader, "file00.wav")]}, True, - ), # FIXME + ), ( "./test-data/example-bundles/ex010", {"file00": [(TextGridLoader, "sub00/file00.TextGrid")]}, @@ -137,9 +137,9 @@ def test_build_from_nonexistent_dir(mocker) -> None: ), ( "./test-data/example-bundles/ex013", - {"file00": [(TextGridLoader, "sub00/file00.TextGrid")]}, + {"file00": [(TextGridLoader, "sub01/file00.TextGrid")]}, True, - ), # FIXME + ), # ("./test-data/example-bundles/ex014", FIXME), # ("./test-data/example-bundles/ex015", FIXME), # ("./test-data/example-bundles/ex016", FIXME), From f7f24b6933885d76c3642f4c37f70c5b61794370 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Mon, 30 Dec 2019 22:18:34 -0800 Subject: [PATCH 08/15] FileBundleList: Add symlink tests --- ultratrace2/model/files/tests/test_bundle.py | 42 +++++++++++++++++--- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/ultratrace2/model/files/tests/test_bundle.py b/ultratrace2/model/files/tests/test_bundle.py index 7fef02a..6bb730c 100644 --- a/ultratrace2/model/files/tests/test_bundle.py +++ b/ultratrace2/model/files/tests/test_bundle.py @@ -1,9 +1,16 @@ +from typing import Dict, Mapping, Sequence, Tuple, Type + import os import pytest from ..bundle import FileBundle, FileBundleList from ..loaders import DICOMLoader, MP3Loader, TextGridLoader, WAVLoader -from ..loaders.base import AlignmentFileLoader, ImageSetFileLoader, SoundFileLoader +from ..loaders.base import ( + FileLoaderBase, + AlignmentFileLoader, + ImageSetFileLoader, + SoundFileLoader, +) @pytest.mark.parametrize( @@ -19,7 +26,7 @@ dict(alignment_file=None, image_set_file=None, sound_file=None), ], ) -def test_empty_file_bundle_constructor(kwargs) -> None: +def test_empty_file_bundle_constructor(kwargs: Mapping[str, None]) -> None: fb = FileBundle("test", **kwargs) assert not fb.has_impl() assert str(fb) == 'Bundle("test",None,None,None)' @@ -140,13 +147,36 @@ def test_build_from_nonexistent_dir(mocker) -> None: {"file00": [(TextGridLoader, "sub01/file00.TextGrid")]}, True, ), - # ("./test-data/example-bundles/ex014", FIXME), - # ("./test-data/example-bundles/ex015", FIXME), - # ("./test-data/example-bundles/ex016", FIXME), + ( + "./test-data/example-bundles/ex014", + {"link00": [(TextGridLoader, "../ex004/file00.TextGrid")]}, + False, + ), + ( + "./test-data/example-bundles/ex015", + { + "file00": [(MP3Loader, "file00.mp3")], + "link00": [(TextGridLoader, "../ex004/file00.TextGrid")], + }, + False, + ), + ( + "./test-data/example-bundles/ex016", + { + "link00": [ + (MP3Loader, "link00.mp3"), + (TextGridLoader, "../ex004/file00.TextGrid"), + ] + }, + False, + ), ], ) def test_build_from_dir( - mocker, source_dir, expected_file_map, should_emit_warning + mocker, + source_dir: str, + expected_file_map: Dict[str, Sequence[Tuple[Type[FileLoaderBase], str]]], + should_emit_warning: bool, ) -> None: mock_file_bundle_list_constructor = mocker.patch( "ultratrace2.model.files.bundle.FileBundleList.__init__", return_value=None, From 7e1ffdb1eddca6b3a39feee43b51c565e41f309b Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Mon, 30 Dec 2019 22:26:57 -0800 Subject: [PATCH 09/15] FLACLoader: Add "audio/x-flac" mime type This also adds tests from the ftyers/ directory. --- ultratrace2/model/files/__init__.py | 2 +- ultratrace2/model/files/tests/test_bundle.py | 13 ++++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/ultratrace2/model/files/__init__.py b/ultratrace2/model/files/__init__.py index 62d754b..e5a9908 100644 --- a/ultratrace2/model/files/__init__.py +++ b/ultratrace2/model/files/__init__.py @@ -19,7 +19,7 @@ from .loaders import FLACLoader __register( - [".flac"], ["audio/flac"], FLACLoader, + [".flac"], ["audio/flac", "audio/x-flac"], FLACLoader, ) except ImportError as e: logger.warning(e) diff --git a/ultratrace2/model/files/tests/test_bundle.py b/ultratrace2/model/files/tests/test_bundle.py index 6bb730c..ac49cad 100644 --- a/ultratrace2/model/files/tests/test_bundle.py +++ b/ultratrace2/model/files/tests/test_bundle.py @@ -4,7 +4,7 @@ import pytest from ..bundle import FileBundle, FileBundleList -from ..loaders import DICOMLoader, MP3Loader, TextGridLoader, WAVLoader +from ..loaders import DICOMLoader, FLACLoader, MP3Loader, TextGridLoader, WAVLoader from ..loaders.base import ( FileLoaderBase, AlignmentFileLoader, @@ -170,6 +170,17 @@ def test_build_from_nonexistent_dir(mocker) -> None: }, False, ), + ( + "./test-data/ftyers", + { + "20150629171639": [ + (DICOMLoader, "20150629171639.dicom"), + (FLACLoader, "20150629171639.flac"), + (TextGridLoader, "20150629171639.TextGrid"), + ], + }, + True, + ), ], ) def test_build_from_dir( From 72202c6eab0f7e8df0725f53528cbeb9923d6535 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Tue, 31 Dec 2019 08:30:46 -0800 Subject: [PATCH 10/15] FileBundle: Add tests for object construction --- ultratrace2/model/files/bundle.py | 9 +++ ultratrace2/model/files/tests/test_bundle.py | 72 ++++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/ultratrace2/model/files/bundle.py b/ultratrace2/model/files/bundle.py index 2a6c705..49870cc 100644 --- a/ultratrace2/model/files/bundle.py +++ b/ultratrace2/model/files/bundle.py @@ -35,16 +35,25 @@ def has_impl(self) -> bool: for f in [self.alignment_file, self.image_set_file, self.sound_file] ) + def get_alignment_file(self) -> Optional[AlignmentFileLoader]: + return self.alignment_file + def set_alignment_file(self, alignment_file: AlignmentFileLoader) -> None: if self.alignment_file is not None: logger.warning("Overwriting existing alignment file") self.alignment_file = alignment_file + def get_image_set_file(self) -> Optional[ImageSetFileLoader]: + return self.image_set_file + def set_image_set_file(self, image_set_file: ImageSetFileLoader) -> None: if self.image_set_file is not None: logger.warning("Overwriting existing image-set file") self.image_set_file = image_set_file + def get_sound_file(self) -> Optional[SoundFileLoader]: + return self.sound_file + def set_sound_file(self, sound_file: SoundFileLoader) -> None: if self.sound_file is not None: logger.warning("Overwriting existing sound file") diff --git a/ultratrace2/model/files/tests/test_bundle.py b/ultratrace2/model/files/tests/test_bundle.py index ac49cad..6881191 100644 --- a/ultratrace2/model/files/tests/test_bundle.py +++ b/ultratrace2/model/files/tests/test_bundle.py @@ -32,6 +32,78 @@ def test_empty_file_bundle_constructor(kwargs: Mapping[str, None]) -> None: assert str(fb) == 'Bundle("test",None,None,None)' +@pytest.mark.parametrize( + "kwargs", + [ + dict( + alignment_file=TextGridLoader.from_file( + "./test-data/example-bundles/ex000/file00.TextGrid" + ) + ), + dict( + image_set_file=DICOMLoader.from_file( + "./test-data/example-bundles/ex004/file00.dicom" + ) + ), + dict( + sound_file=MP3Loader.from_file( + "./test-data/example-bundles/ex002/file00.mp3" + ) + ), + dict( + alignment_file=TextGridLoader.from_file( + "./test-data/example-bundles/ex000/file00.TextGrid" + ), + image_set_file=DICOMLoader.from_file( + "./test-data/example-bundles/ex004/file00.dicom" + ), + ), + dict( + alignment_file=TextGridLoader.from_file( + "./test-data/example-bundles/ex000/file00.TextGrid" + ), + sound_file=MP3Loader.from_file( + "./test-data/example-bundles/ex002/file00.mp3" + ), + ), + dict( + image_set_file=DICOMLoader.from_file( + "./test-data/example-bundles/ex004/file00.dicom" + ), + sound_file=MP3Loader.from_file( + "./test-data/example-bundles/ex002/file00.mp3" + ), + ), + dict( + alignment_file=TextGridLoader.from_file( + "./test-data/example-bundles/ex000/file00.TextGrid" + ), + image_set_file=DICOMLoader.from_file( + "./test-data/example-bundles/ex004/file00.dicom" + ), + sound_file=MP3Loader.from_file( + "./test-data/example-bundles/ex002/file00.mp3" + ), + ), + ], +) +def test_file_bundle_constructor(kwargs: Mapping[str, FileLoaderBase]) -> None: + fb = FileBundle("test", **kwargs) + assert fb.has_impl() + if "alignment_file" in kwargs: + alignment_file = fb.get_alignment_file() + assert isinstance(alignment_file, AlignmentFileLoader) + assert alignment_file.get_path() == kwargs["alignment_file"].get_path() + if "image_set_file" in kwargs: + image_set_file = fb.get_image_set_file() + assert isinstance(image_set_file, ImageSetFileLoader) + assert image_set_file.get_path() == kwargs["image_set_file"].get_path() + if "sound_file" in kwargs: + sound_file = fb.get_sound_file() + assert isinstance(sound_file, SoundFileLoader) + assert sound_file.get_path() == kwargs["sound_file"].get_path() + + def test_build_from_nonexistent_dir(mocker) -> None: mock_file_bundle_list_constructor = mocker.patch( "ultratrace2.model.files.bundle.FileBundleList.__init__", return_value=None, From bf3b738fdef07a2e13ed69c6bab454407b98e38d Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Tue, 31 Dec 2019 08:37:43 -0800 Subject: [PATCH 11/15] TextGridLoader: Make @classmethods @staticmethods (#118) --- ultratrace2/model/files/loaders/textgrid.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ultratrace2/model/files/loaders/textgrid.py b/ultratrace2/model/files/loaders/textgrid.py index 30f2cc7..1048633 100644 --- a/ultratrace2/model/files/loaders/textgrid.py +++ b/ultratrace2/model/files/loaders/textgrid.py @@ -99,15 +99,15 @@ def from_file(cls, path: str) -> "TextGridLoader": f"Invalid TextGrid ({path}), unable to read: {str(e)}" ) from e - @classmethod - def load_with_encoding(cls, path: str, encoding: str) -> textgrid.TextGrid: + @staticmethod + def load_with_encoding(path: str, encoding: str) -> textgrid.TextGrid: if encoding == "utf-8" or encoding == "utf-16": return textgrid.TextGrid.fromFile(path) else: - transcoded_file = cls.copy_to_temp_file_with_encoding(path, encoding) + transcoded_file = TextGridLoader.copy_to_temp_file_with_encoding(path, encoding) return textgrid.TextGrid.fromFile(transcoded_file.name) - @classmethod + @staticmethod def copy_to_temp_file_with_encoding( cls, original_path: str, encoding: str ) -> IO[bytes]: From e8921643a4136d12cca09a7a26e3008e18e3ab58 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Tue, 31 Dec 2019 08:38:44 -0800 Subject: [PATCH 12/15] TextGridLoader: Explicitly manage our temporary file contexts Before, we were relying on garbage collection to clean up our temp files, which is probably not tenable long-term. --- ultratrace2/model/files/loaders/textgrid.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/ultratrace2/model/files/loaders/textgrid.py b/ultratrace2/model/files/loaders/textgrid.py index 1048633..ca10d15 100644 --- a/ultratrace2/model/files/loaders/textgrid.py +++ b/ultratrace2/model/files/loaders/textgrid.py @@ -104,17 +104,17 @@ def load_with_encoding(path: str, encoding: str) -> textgrid.TextGrid: if encoding == "utf-8" or encoding == "utf-16": return textgrid.TextGrid.fromFile(path) else: - transcoded_file = TextGridLoader.copy_to_temp_file_with_encoding(path, encoding) - return textgrid.TextGrid.fromFile(transcoded_file.name) + with tempfile.NamedTemporaryFile() as temp_file: + TextGridLoader.copy_to_temp_file_with_encoding( + path, temp_file, encoding + ) + return textgrid.TextGrid.fromFile(temp_file.name) @staticmethod def copy_to_temp_file_with_encoding( - cls, original_path: str, encoding: str - ) -> IO[bytes]: - # this gets ::close()d when it is GC'ed - temp_file = tempfile.NamedTemporaryFile() + original_path: str, temp_file: IO[bytes], encoding: str + ) -> None: with open(original_path, "rb") as orig_file: transcoded_contents = orig_file.read().decode(encoding).encode("utf-8") temp_file.write(transcoded_contents) temp_file.seek(0) - return temp_file From fca32594e19f0b562c3c52e9acafcf2cbce9a772 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Tue, 31 Dec 2019 08:49:00 -0800 Subject: [PATCH 13/15] PydubLoader: Add helper class for SoundFileLoader to avoid repetition There needs to be some class sitting between SoundFileLoader and the concrete children since - each child shares a lot of code with siblings - each child needs the `pydub` external library, which may not exist on all systems See #122 for more context. --- ultratrace2/model/files/loaders/flac.py | 29 ++---------------------- ultratrace2/model/files/loaders/mp3.py | 29 ++---------------------- ultratrace2/model/files/loaders/ogg.py | 29 ++---------------------- ultratrace2/model/files/loaders/pydub.py | 26 +++++++++++++++++++++ ultratrace2/model/files/loaders/wav.py | 29 ++---------------------- 5 files changed, 34 insertions(+), 108 deletions(-) create mode 100644 ultratrace2/model/files/loaders/pydub.py diff --git a/ultratrace2/model/files/loaders/flac.py b/ultratrace2/model/files/loaders/flac.py index d307ac9..d41ba5f 100644 --- a/ultratrace2/model/files/loaders/flac.py +++ b/ultratrace2/model/files/loaders/flac.py @@ -1,32 +1,7 @@ -import pydub # type: ignore +from .pydub import PydubLoader -from .base import FileLoadError, SoundFileLoader - - -class FLACLoader(SoundFileLoader): - def get_path(self) -> str: - return self._path - - def set_path(self, path) -> None: - self._path = path - - def __init__(self, path: str, audio_segment: pydub.AudioSegment): - self.set_path(path) - self.audio_segment = audio_segment - - def __len__(self) -> int: - return len(self.audio_segment) - - @classmethod - def from_file(cls, path: str) -> "FLACLoader": - try: - audio_segment = pydub.AudioSegment.from_file(path) - return FLACLoader(path, audio_segment) - except Exception as e: - raise FileLoadError( - f"Invalid FLAC ({path}), unable to read: {str(e)}" - ) from e +class FLACLoader(PydubLoader): @staticmethod def get_priority() -> int: return 4 diff --git a/ultratrace2/model/files/loaders/mp3.py b/ultratrace2/model/files/loaders/mp3.py index 3187dd2..8e6a537 100644 --- a/ultratrace2/model/files/loaders/mp3.py +++ b/ultratrace2/model/files/loaders/mp3.py @@ -1,32 +1,7 @@ -import pydub # type: ignore +from .pydub import PydubLoader -from .base import FileLoadError, SoundFileLoader - - -class MP3Loader(SoundFileLoader): - def get_path(self) -> str: - return self._path - - def set_path(self, path) -> None: - self._path = path - - def __init__(self, path: str, audio_segment: pydub.AudioSegment): - self.set_path(path) - self.audio_segment = audio_segment - - def __len__(self) -> int: - return len(self.audio_segment) - - @classmethod - def from_file(cls, path: str) -> "MP3Loader": - try: - audio_segment = pydub.AudioSegment.from_file(path) - return MP3Loader(path, audio_segment) - except Exception as e: - raise FileLoadError( - f"Invalid MP3 ({path}), unable to read: {str(e)}" - ) from e +class MP3Loader(PydubLoader): @staticmethod def get_priority() -> int: return 1 diff --git a/ultratrace2/model/files/loaders/ogg.py b/ultratrace2/model/files/loaders/ogg.py index 15d57ac..80bcdb6 100644 --- a/ultratrace2/model/files/loaders/ogg.py +++ b/ultratrace2/model/files/loaders/ogg.py @@ -1,32 +1,7 @@ -import pydub # type: ignore +from .pydub import PydubLoader -from .base import FileLoadError, SoundFileLoader - - -class OggLoader(SoundFileLoader): - def get_path(self) -> str: - return self._path - - def set_path(self, path) -> None: - self._path = path - - def __init__(self, path: str, audio_segment: pydub.AudioSegment): - self.set_path(path) - self.audio_segment = audio_segment - - def __len__(self) -> int: - return len(self.audio_segment) - - @classmethod - def from_file(cls, path: str) -> "OggLoader": - try: - audio_segment = pydub.AudioSegment.from_file(path) - return OggLoader(path, audio_segment) - except Exception as e: - raise FileLoadError( - f"Invalid Ogg ({path}), unable to read: {str(e)}" - ) from e +class OggLoader(PydubLoader): @staticmethod def get_priority() -> int: return 2 diff --git a/ultratrace2/model/files/loaders/pydub.py b/ultratrace2/model/files/loaders/pydub.py new file mode 100644 index 0000000..470878c --- /dev/null +++ b/ultratrace2/model/files/loaders/pydub.py @@ -0,0 +1,26 @@ +import pydub # type: ignore + +from .base import FileLoadError, SoundFileLoader + + +class PydubLoader(SoundFileLoader): + def get_path(self) -> str: + return self._path + + def set_path(self, path) -> None: + self._path = path + + def __init__(self, path: str, audio_segment: pydub.AudioSegment): + self.set_path(path) + self.audio_segment = audio_segment + + def __len__(self) -> int: + return len(self.audio_segment) + + @classmethod + def from_file(cls, path: str) -> "PydubLoader": + try: + audio_segment = pydub.AudioSegment.from_file(path) + return PydubLoader(path, audio_segment) + except Exception as e: + raise FileLoadError(f"Invalid AudioSegment ({path}), unable to read") from e diff --git a/ultratrace2/model/files/loaders/wav.py b/ultratrace2/model/files/loaders/wav.py index 74ea52a..1dcdce6 100644 --- a/ultratrace2/model/files/loaders/wav.py +++ b/ultratrace2/model/files/loaders/wav.py @@ -1,32 +1,7 @@ -import pydub # type: ignore +from .pydub import PydubLoader -from .base import FileLoadError, SoundFileLoader - - -class WAVLoader(SoundFileLoader): - def get_path(self) -> str: - return self._path - - def set_path(self, path) -> None: - self._path = path - - def __init__(self, path: str, audio_segment: pydub.AudioSegment): - self.set_path(path) - self.audio_segment = audio_segment - - def __len__(self) -> int: - return len(self.audio_segment) - - @classmethod - def from_file(cls, path: str) -> "WAVLoader": - try: - audio_segment = pydub.AudioSegment.from_file(path) - return WAVLoader(path, audio_segment) - except Exception as e: - raise FileLoadError( - f"Invalid WAV ({path}), unable to read: {str(e)}" - ) from e +class WAVLoader(PydubLoader): @staticmethod def get_priority() -> int: return 3 From c36ad55f88579cbb7ad31861c8b1b6558c4e0265 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Tue, 31 Dec 2019 09:03:27 -0800 Subject: [PATCH 14/15] SoundFileLoader: Fix typo in test function signatures --- ultratrace2/model/files/loaders/tests/test_flac.py | 2 +- ultratrace2/model/files/loaders/tests/test_mp3.py | 2 +- ultratrace2/model/files/loaders/tests/test_ogg.py | 2 +- ultratrace2/model/files/loaders/tests/test_wav.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ultratrace2/model/files/loaders/tests/test_flac.py b/ultratrace2/model/files/loaders/tests/test_flac.py index bc5c65c..54f79c1 100644 --- a/ultratrace2/model/files/loaders/tests/test_flac.py +++ b/ultratrace2/model/files/loaders/tests/test_flac.py @@ -22,6 +22,6 @@ def test_loading_from_invalid_file(path) -> None: ("./test-data/ftyers/20150629171639.flac", 28200), ], ) -def test_loading_from_valid_file(path: str, duration_ms: float) -> None: +def test_loading_from_valid_file(path: str, duration_ms: int) -> None: flac_file = FLACLoader.from_file(path) assert len(flac_file) == duration_ms diff --git a/ultratrace2/model/files/loaders/tests/test_mp3.py b/ultratrace2/model/files/loaders/tests/test_mp3.py index ee6ea42..9fd85f9 100644 --- a/ultratrace2/model/files/loaders/tests/test_mp3.py +++ b/ultratrace2/model/files/loaders/tests/test_mp3.py @@ -22,6 +22,6 @@ def test_loading_from_invalid_file(path) -> None: ("./test-data/example-audio/pno-cs.mp3", 20064), ], ) -def test_loading_from_valid_file(path: str, duration_ms: float) -> None: +def test_loading_from_valid_file(path: str, duration_ms: int) -> None: mp3_file = MP3Loader.from_file(path) assert len(mp3_file) == duration_ms diff --git a/ultratrace2/model/files/loaders/tests/test_ogg.py b/ultratrace2/model/files/loaders/tests/test_ogg.py index cf70ab7..7d354d2 100644 --- a/ultratrace2/model/files/loaders/tests/test_ogg.py +++ b/ultratrace2/model/files/loaders/tests/test_ogg.py @@ -21,6 +21,6 @@ def test_loading_from_invalid_file(path) -> None: ("./test-data/example-audio/massenet_le_cid.ogg", 261078), ], ) -def test_loading_from_valid_file(path: str, duration_ms: float) -> None: +def test_loading_from_valid_file(path: str, duration_ms: int) -> None: ogg_file = OggLoader.from_file(path) assert len(ogg_file) == duration_ms diff --git a/ultratrace2/model/files/loaders/tests/test_wav.py b/ultratrace2/model/files/loaders/tests/test_wav.py index 6f57278..fb48c4c 100644 --- a/ultratrace2/model/files/loaders/tests/test_wav.py +++ b/ultratrace2/model/files/loaders/tests/test_wav.py @@ -167,6 +167,6 @@ def test_loading_from_invalid_file(path) -> None: ("./test-data/qumuq/File068_Track1.wav", 3390), ], ) -def test_loading_from_valid_file(path: str, duration_ms: float) -> None: +def test_loading_from_valid_file(path: str, duration_ms: int) -> None: wav_file = WAVLoader.from_file(path) assert len(wav_file) == duration_ms From 206191bf44890011db2b858ee412a001b0cd36dc Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Sun, 26 Jan 2020 14:12:29 -0800 Subject: [PATCH 15/15] __main__: Remove duplicate --theme argument --- ultratrace2/__main__.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/ultratrace2/__main__.py b/ultratrace2/__main__.py index a5138f5..2d73455 100644 --- a/ultratrace2/__main__.py +++ b/ultratrace2/__main__.py @@ -22,11 +22,6 @@ def main(): default=None, help="path (unique to a participant) where subdirectories contain raw data", ) - parser.add_argument( - "theme", # FIXME: not yet supported - default=None, - help="name of Ttk theme to use for widgets", - ) parser.add_argument( "--no-audio", dest="audio", @@ -57,7 +52,9 @@ def main(): action="store_false", help="don't try to load the video widget", ) - parser.add_argument("--theme", help="TtkTheme to use for application") + parser.add_argument( + "--theme", help="name of Ttk theme to use for widgets", + ) parser.add_argument( "--max-undo-memory", type=int,