From 05f13c48a3ee2d437397d037fd404ecc7a28c5ca Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Tue, 17 Dec 2019 19:00:28 -0800 Subject: [PATCH 01/35] App: implement managing multiple projects (#107) See the issue for more details --- ultratrace2/app.py | 38 ++++++++++++++++++++++++++++--- ultratrace2/model/files/bundle.py | 10 ++++---- ultratrace2/model/project.py | 23 ++++++++++++++++--- 3 files changed, 61 insertions(+), 10 deletions(-) diff --git a/ultratrace2/app.py b/ultratrace2/app.py index f3d0a14..636bbb5 100644 --- a/ultratrace2/app.py +++ b/ultratrace2/app.py @@ -1,5 +1,8 @@ +import os +import pickle + from tkinter.filedialog import askdirectory as choose_dir -from typing import Optional +from typing import Optional, Set from .gui import GUI from .model.project import Project @@ -15,10 +18,39 @@ def __init__(self, args): # FIXME: be more granular here else: path = args.path - self.project = Project(path) + # FIXME: allow changing the $HOME-as-root via command line arg? + self.ultratrace_home = os.path.join(os.environ["HOME"], ".ultratrace") + if not os.path.exists(self.ultratrace_home): + os.mkdir(self.ultratrace_home) + + self.project_hashes: Set[int] + if os.path.exists(self.get_projects_path()): + with open(self.get_projects_path(), "rb") as fp: + project_hashes = pickle.load(fp) + assert isinstance(project_hashes, set) + for project_hash in project_hashes: + assert isinstance(project_hash, int) + self.project_hashes = project_hashes + else: + self.project_hashes = set() + + self.project: Project = self.get_project_by_path(path) self.gui = GUI(theme=args.theme) - def main(self): + def get_projects_path(self) -> str: + return os.path.join(self.ultratrace_home, "projects.pkl") + + def get_project_path(self, project_hash: int) -> str: + return os.path.join(self.ultratrace_home, "projects", str(project_hash)) + + def get_project_by_path(self, path: str) -> Project: + project_hash = hash(path) + if project_hash in self.project_hashes: + project_path = self.get_project_path(project_hash) + return Project.load(project_path) + return Project.initialize_from_path(path) + + def main(self) -> None: pass diff --git a/ultratrace2/model/files/bundle.py b/ultratrace2/model/files/bundle.py index 9a568fb..0e3c682 100644 --- a/ultratrace2/model/files/bundle.py +++ b/ultratrace2/model/files/bundle.py @@ -1,7 +1,7 @@ import logging import os -from typing import Dict, List, Set +from typing import Dict, List, Sequence, Set from .impls import Sound, Alignment, ImageSet @@ -45,25 +45,27 @@ class FileBundleList: ] ) - def __init__(self, path: str, extra_exclude_dirs: List[str] = []): + def __init__(self, extra_exclude_dirs: Sequence[str] = []): # FIXME: implement `extra_exclude_dirs` as a command-line arg for extra_exclude_dir in extra_exclude_dirs: self.exclude_dirs.add(extra_exclude_dir) - self.path = path self.has_alignment_impl = False self.has_image_impl = False self.has_sound_impl = False self.current_bundle = None + self.bundles: List[FileBundle] = [] # FIXME: build up this data structure + + def scan_dir(self, root_path): bundles: Dict[str, FileBundle] = {} # NB: `topdown=True` increases runtime cost from O(n) -> O(n^2), but it allows us to # modify `dirs` in-place so that we can skip certain directories. For more info, # see https://stackoverflow.com/questions/19859840/excluding-directories-in-os-walk - for path, dirs, filenames in os.walk(path, topdown=True): + for path, dirs, filenames in os.walk(root_path, topdown=True): dirs[:] = [d for d in dirs if d not in self.exclude_dirs] for filename in filenames: diff --git a/ultratrace2/model/project.py b/ultratrace2/model/project.py index 78a8f09..2819948 100644 --- a/ultratrace2/model/project.py +++ b/ultratrace2/model/project.py @@ -1,4 +1,5 @@ import os +import pickle from .trace import TraceList from .files.bundle import FileBundleList @@ -6,18 +7,34 @@ class Project: def __init__(self, path: str): + """ + Internal function: to construct a Project, either call via ::initialize_from_path() + or ::load(). + """ if not os.path.exists(path): raise ValueError(f"cannot initialize project at {path}") self.root_path = os.path.realpath(os.path.abspath(path)) # absolute path self.traces = TraceList() - self.files = FileBundleList(self.root_path) + self.files = FileBundleList() def save(self): raise NotImplementedError() @classmethod - def load(cls): - raise NotImplementedError() + def load(cls, project_path) -> "Project": + with open(project_path, "rb") as fp: + project = pickle.load(fp) + assert isinstance(project, Project) + return project + + @classmethod + def initialize_from_path(cls, data_path) -> "Project": + project = Project(data_path) + project.scan_files() + return project + + def scan_files(self) -> None: + self.files.scan_dir(self.root_path) def filepath(self): raise NotImplementedError() From ca46724adf03355e0dbdde2a6a9b6b1e6374f8b8 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Tue, 17 Dec 2019 19:03:50 -0800 Subject: [PATCH 02/35] App: save `project_hash` after initializing a Project --- ultratrace2/app.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ultratrace2/app.py b/ultratrace2/app.py index 636bbb5..cc43439 100644 --- a/ultratrace2/app.py +++ b/ultratrace2/app.py @@ -48,7 +48,9 @@ def get_project_by_path(self, path: str) -> Project: if project_hash in self.project_hashes: project_path = self.get_project_path(project_hash) return Project.load(project_path) - return Project.initialize_from_path(path) + project = Project.initialize_from_path(path) + self.project_hashes.add(project_hash) + return project def main(self) -> None: pass From 7c46ccb94bf866a69bef140d71a2d4a29de64b24 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 15:16:25 -0800 Subject: [PATCH 03/35] Project: change constructor pattern & save location Following feedback on #107, this patch implements saving the ultratrace metadata directly in the directory containing the source files. In addition, it changes the way we construct Projects and FileBundleLists so that we avoid half-constructed objects. --- ultratrace2/app.py | 38 +------------------ ultratrace2/model/files/bundle.py | 47 +++++++++++++----------- ultratrace2/model/project.py | 61 ++++++++++++++++++++++--------- 3 files changed, 72 insertions(+), 74 deletions(-) diff --git a/ultratrace2/app.py b/ultratrace2/app.py index cc43439..adef47f 100644 --- a/ultratrace2/app.py +++ b/ultratrace2/app.py @@ -1,8 +1,5 @@ -import os -import pickle - from tkinter.filedialog import askdirectory as choose_dir -from typing import Optional, Set +from typing import Optional from .gui import GUI from .model.project import Project @@ -18,40 +15,9 @@ def __init__(self, args): # FIXME: be more granular here else: path = args.path - # FIXME: allow changing the $HOME-as-root via command line arg? - self.ultratrace_home = os.path.join(os.environ["HOME"], ".ultratrace") - if not os.path.exists(self.ultratrace_home): - os.mkdir(self.ultratrace_home) - - self.project_hashes: Set[int] - if os.path.exists(self.get_projects_path()): - with open(self.get_projects_path(), "rb") as fp: - project_hashes = pickle.load(fp) - assert isinstance(project_hashes, set) - for project_hash in project_hashes: - assert isinstance(project_hash, int) - self.project_hashes = project_hashes - else: - self.project_hashes = set() - - self.project: Project = self.get_project_by_path(path) + self.project: Project = Project.get_by_path(path) self.gui = GUI(theme=args.theme) - def get_projects_path(self) -> str: - return os.path.join(self.ultratrace_home, "projects.pkl") - - def get_project_path(self, project_hash: int) -> str: - return os.path.join(self.ultratrace_home, "projects", str(project_hash)) - - def get_project_by_path(self, path: str) -> Project: - project_hash = hash(path) - if project_hash in self.project_hashes: - project_path = self.get_project_path(project_hash) - return Project.load(project_path) - project = Project.initialize_from_path(path) - self.project_hashes.add(project_hash) - return project - def main(self) -> None: pass diff --git a/ultratrace2/model/files/bundle.py b/ultratrace2/model/files/bundle.py index 0e3c682..3773da3 100644 --- a/ultratrace2/model/files/bundle.py +++ b/ultratrace2/model/files/bundle.py @@ -1,7 +1,7 @@ import logging import os -from typing import Dict, List, Sequence, Set +from typing import Dict, Sequence, Set from .impls import Sound, Alignment, ImageSet @@ -41,24 +41,37 @@ class FileBundleList: ".git", "node_modules", "__pycache__", + ".ultratrace", # FIXME: add more ignoreable dirs ] ) - def __init__(self, extra_exclude_dirs: Sequence[str] = []): + def __init__(self, bundles: Dict[str, FileBundle]): - # FIXME: implement `extra_exclude_dirs` as a command-line arg - for extra_exclude_dir in extra_exclude_dirs: - self.exclude_dirs.add(extra_exclude_dir) + self.current_bundle = None + self.bundles: Dict[str, FileBundle] = bundles - self.has_alignment_impl = False - self.has_image_impl = False - self.has_sound_impl = False + self.has_alignment_impl: bool = False + self.has_image_impl: bool = False + self.has_sound_impl: bool = False - self.current_bundle = None - self.bundles: List[FileBundle] = [] # FIXME: build up this data structure + # FIXME: do this when we add to our data structure + for filename, bundle in bundles.items(): + # build up self.bundles here + if not self.has_alignment_impl and bundle.alignment_file.has_impl(): + self.has_alignment_impl = True + if not self.has_image_impl and bundle.image_file.has_impl(): + self.has_image_impl = True + if not self.has_sound_impl and bundle.sound_file.has_impl(): + self.has_sound_impl = True - def scan_dir(self, root_path): + @classmethod + def build_from_dir( + cls, root_path: str, extra_exclude_dirs: Sequence[str] = [] + ) -> "FileBundleList": + + # FIXME: implement `extra_exclude_dirs` as a command-line arg + exclude_dirs = cls.exclude_dirs.copy().union(extra_exclude_dirs) bundles: Dict[str, FileBundle] = {} @@ -66,7 +79,7 @@ def scan_dir(self, root_path): # modify `dirs` in-place so that we can skip certain directories. For more info, # see https://stackoverflow.com/questions/19859840/excluding-directories-in-os-walk for path, dirs, filenames in os.walk(root_path, topdown=True): - dirs[:] = [d for d in dirs if d not in self.exclude_dirs] + dirs[:] = [d for d in dirs if d not in exclude_dirs] for filename in filenames: @@ -85,12 +98,4 @@ def scan_dir(self, root_path): if not bundles[name].interpret(filepath): logger.warning(f"unrecognized filetype: {filepath}") - # FIXME: do this when we add to our data structure - for filename, bundle in bundles.items(): - # build up self.bundles here - if not self.has_alignment_impl and bundle.alignment_file.has_impl(): - self.has_alignment_impl = True - if not self.has_image_impl and bundle.image_file.has_impl(): - self.has_image_impl = True - if not self.has_sound_impl and bundle.sound_file.has_impl(): - self.has_sound_impl = True + return FileBundleList(bundles) diff --git a/ultratrace2/model/project.py b/ultratrace2/model/project.py index 2819948..0f4aa2f 100644 --- a/ultratrace2/model/project.py +++ b/ultratrace2/model/project.py @@ -1,40 +1,67 @@ +import logging import os import pickle from .trace import TraceList from .files.bundle import FileBundleList +logger = logging.getLogger(__name__) + class Project: - def __init__(self, path: str): + def __init__(self, traces: TraceList, files: FileBundleList): """ Internal function: to construct a Project, either call via ::initialize_from_path() or ::load(). """ - if not os.path.exists(path): - raise ValueError(f"cannot initialize project at {path}") - self.root_path = os.path.realpath(os.path.abspath(path)) # absolute path - self.traces = TraceList() - self.files = FileBundleList() + self.traces = traces + self.files = files def save(self): raise NotImplementedError() @classmethod - def load(cls, project_path) -> "Project": - with open(project_path, "rb") as fp: - project = pickle.load(fp) - assert isinstance(project, Project) - return project + def load(cls, save_file) -> "Project": + try: + with open(save_file, "rb") as fp: + project = pickle.load(fp) + assert isinstance(project, Project) + return project + except Exception as e: + logger.error(e) + raise RuntimeError(str(e)) @classmethod - def initialize_from_path(cls, data_path) -> "Project": - project = Project(data_path) - project.scan_files() - return project + def get_by_path(cls, root_path) -> "Project": + + root_path = os.path.realpath(os.path.abspath(root_path)) # absolute path + if not os.path.exists(root_path): + raise ValueError(f"cannot initialize project at {root_path}") + + save_dir = Project.get_save_dir(root_path) + if not os.path.exists(save_dir): + os.mkdir(save_dir) + + save_file = Project.get_save_file(root_path) + try: + return Project.load(save_file) + except RuntimeError: + logger.info( + f"Unable to find existing project at {root_path}, creating new one..." + ) + + traces = TraceList() + file_bundles = FileBundleList.build_from_dir(root_path) + return Project(traces, file_bundles) + + @staticmethod + def get_save_dir(path: str) -> str: + return os.path.join(path, ".ultratrace") - def scan_files(self) -> None: - self.files.scan_dir(self.root_path) + @staticmethod + def get_save_file(path: str) -> str: + save_dir = Project.get_save_dir(path) + return os.path.join(save_dir, "project.pkl") def filepath(self): raise NotImplementedError() From 1c15acab112848a4cfa4784b7c6b51da228976cf Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 15:23:02 -0800 Subject: [PATCH 04/35] Create directories in 0o755 mode (instead of default 0o777) --- ultratrace2/model/files/impls/image.py | 2 +- ultratrace2/model/project.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ultratrace2/model/files/impls/image.py b/ultratrace2/model/files/impls/image.py index 11aff06..1979559 100644 --- a/ultratrace2/model/files/impls/image.py +++ b/ultratrace2/model/files/impls/image.py @@ -54,7 +54,7 @@ def load(self) -> None: "Invalid DICOM ({self.path}), unknown shape {pixels.shape}" ) - os.mkdir(self.png_path) + os.mkdir(self.png_path, mode=0o755) for i in tqdm(range(frames), desc="converting to PNG"): filename = os.path.join(self.png_path, f"{i:06}.png") arr = pixels[i, :, :] if is_greyscale else pixels[i, :, :, :] diff --git a/ultratrace2/model/project.py b/ultratrace2/model/project.py index 0f4aa2f..b084f7a 100644 --- a/ultratrace2/model/project.py +++ b/ultratrace2/model/project.py @@ -40,7 +40,7 @@ def get_by_path(cls, root_path) -> "Project": save_dir = Project.get_save_dir(root_path) if not os.path.exists(save_dir): - os.mkdir(save_dir) + os.mkdir(save_dir, mode=0o755) save_file = Project.get_save_file(root_path) try: From fd59bfd65b82ec1a9c3b78d38febbd25a89a3e5c Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 15:32:34 -0800 Subject: [PATCH 05/35] Project: remove old comments --- ultratrace2/model/files/bundle.py | 2 -- ultratrace2/model/project.py | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/ultratrace2/model/files/bundle.py b/ultratrace2/model/files/bundle.py index 3773da3..56f8440 100644 --- a/ultratrace2/model/files/bundle.py +++ b/ultratrace2/model/files/bundle.py @@ -55,9 +55,7 @@ def __init__(self, bundles: Dict[str, FileBundle]): self.has_image_impl: bool = False self.has_sound_impl: bool = False - # FIXME: do this when we add to our data structure for filename, bundle in bundles.items(): - # build up self.bundles here if not self.has_alignment_impl and bundle.alignment_file.has_impl(): self.has_alignment_impl = True if not self.has_image_impl and bundle.image_file.has_impl(): diff --git a/ultratrace2/model/project.py b/ultratrace2/model/project.py index b084f7a..d82f409 100644 --- a/ultratrace2/model/project.py +++ b/ultratrace2/model/project.py @@ -11,8 +11,7 @@ class Project: def __init__(self, traces: TraceList, files: FileBundleList): """ - Internal function: to construct a Project, either call via ::initialize_from_path() - or ::load(). + Internal function: to construct a Project, either call ::get_by_path() """ self.traces = traces self.files = files From b2d62bfece4ccf6d7573fcee692b8264024143fd Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 15:32:43 -0800 Subject: [PATCH 06/35] Project: check if root_path is a directory --- ultratrace2/model/project.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ultratrace2/model/project.py b/ultratrace2/model/project.py index d82f409..24982b2 100644 --- a/ultratrace2/model/project.py +++ b/ultratrace2/model/project.py @@ -34,8 +34,10 @@ def load(cls, save_file) -> "Project": def get_by_path(cls, root_path) -> "Project": root_path = os.path.realpath(os.path.abspath(root_path)) # absolute path - if not os.path.exists(root_path): - raise ValueError(f"cannot initialize project at {root_path}") + if not os.path.exists(root_path) or not os.path.isdir(root_path): + raise ValueError( + f"cannot initialize project at {root_path}: not a directory" + ) save_dir = Project.get_save_dir(root_path) if not os.path.exists(save_dir): From 746f9acd939c7842be926acfdbdceaa7d986db92 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 16:28:44 -0800 Subject: [PATCH 07/35] App: Add types to constructor args, allow "headless" mode --- ultratrace2/app.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/ultratrace2/app.py b/ultratrace2/app.py index adef47f..4192808 100644 --- a/ultratrace2/app.py +++ b/ultratrace2/app.py @@ -1,3 +1,4 @@ +from argparse import Namespace from tkinter.filedialog import askdirectory as choose_dir from typing import Optional @@ -6,17 +7,20 @@ class App: - def __init__(self, args): # FIXME: be more granular here + def __init__(self, args: Namespace): - if args.path is None: + headless = getattr(args, "headless", False) + + path: Optional[str] = getattr(args, "path", None) + if path is None and not headless: path = choose_dir() - if not path: - raise ValueError("You must choose a directory to open") - else: - path = args.path + if not path: + raise ValueError("You must choose a directory to open") self.project: Project = Project.get_by_path(path) - self.gui = GUI(theme=args.theme) + + if not headless: + self.gui = GUI(theme=args.theme) def main(self) -> None: pass @@ -26,7 +30,7 @@ def main(self) -> None: app: Optional[App] = None -def initialize_app(args) -> App: # FIXME: be more granular here +def initialize_app(args: Namespace) -> App: global app app = App(args) return app From 274beae4ba2ead74f037d2f7c26fe123c75c3048 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 16:29:16 -0800 Subject: [PATCH 08/35] App: add tests --- ultratrace2/tests/__init__.py | 0 ultratrace2/tests/test_app.py | 36 +++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 ultratrace2/tests/__init__.py create mode 100644 ultratrace2/tests/test_app.py diff --git a/ultratrace2/tests/__init__.py b/ultratrace2/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/ultratrace2/tests/test_app.py b/ultratrace2/tests/test_app.py new file mode 100644 index 0000000..1002074 --- /dev/null +++ b/ultratrace2/tests/test_app.py @@ -0,0 +1,36 @@ +from argparse import Namespace +from pathlib import Path + +import pytest + +from .. import app as app_py +from ..app import App, initialize_app + + +@pytest.mark.parametrize( + "args,expected", + [ + (Namespace(headless=True), ValueError), + (Namespace(headless=True, path="/path/to/nowhere"), ValueError), + (Namespace(headless=True, path="/dev/null"), ValueError), # not a directory + (Namespace(headless=True, path="/"), PermissionError), # not writeable + ], +) +def test_initialize_app_invalid(args: Namespace, expected: Exception, tmpdir) -> None: + app_py.app = None # overwrite global object + with pytest.raises(expected): + initialize_app(args) + + +@pytest.mark.parametrize("args", [(Namespace(headless=True)),]) # noqa: E231 +def test_initialize_app_valid(args: Namespace, tmp_path: Path) -> None: + # overwrite global object + app_py.app = None + # initialize an empty dir + path = tmp_path / "ultratrace-test-app" + path.mkdir() + # save that to the Namespace + args.path = path + # initialize + app = initialize_app(args) + assert isinstance(app, App) From 30c5c7a89bd297bfecda7ff562b37d73c2dae6b7 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 16:29:32 -0800 Subject: [PATCH 09/35] Trace+XHair: fix type errors when running `pytest` --- ultratrace2/model/trace.py | 4 ++-- ultratrace2/model/xhair.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ultratrace2/model/trace.py b/ultratrace2/model/trace.py index cf40492..ccd31c4 100644 --- a/ultratrace2/model/trace.py +++ b/ultratrace2/model/trace.py @@ -17,7 +17,7 @@ class Trace: def __init__(self, name: str, color: Color): self.id: UUID = uuid4() self.is_visible: bool = True - self.xhairs: Dict[FileBundle, Dict[int, Set[XHair]]] = {} + self.xhairs: Dict["FileBundle", Dict[int, Set[XHair]]] = {} self.name = name self.color = color @@ -45,7 +45,7 @@ def show(self) -> None: def hide(self) -> None: self.is_visible = False - def add_xhair(self, bundle: FileBundle, frame: int, x: float, y: float) -> None: + def add_xhair(self, bundle: "FileBundle", frame: int, x: float, y: float) -> None: if bundle not in self.xhairs: self.xhairs[bundle] = {} if frame not in self.xhairs[bundle]: diff --git a/ultratrace2/model/xhair.py b/ultratrace2/model/xhair.py index 3b69b3e..ac0bdce 100644 --- a/ultratrace2/model/xhair.py +++ b/ultratrace2/model/xhair.py @@ -7,7 +7,7 @@ class XHair: - def __init__(self, trace: Trace, x: float, y: float): + def __init__(self, trace: "Trace", x: float, y: float): self.id = uuid4() self.trace = trace @@ -49,5 +49,5 @@ def move(self, x: float, y: float) -> None: self.x = x self.y = y - def get_color(self) -> Color: + def get_color(self) -> "Color": return self.trace.get_color() From 61a18b60947870ad92658aa5594db3770aaca4dd Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 17:01:59 -0800 Subject: [PATCH 10/35] Update ultratrace2/model/files/bundle.py Co-Authored-By: Caleb --- 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 56f8440..4435877 100644 --- a/ultratrace2/model/files/bundle.py +++ b/ultratrace2/model/files/bundle.py @@ -55,7 +55,7 @@ def __init__(self, bundles: Dict[str, FileBundle]): self.has_image_impl: bool = False self.has_sound_impl: bool = False - for filename, bundle in bundles.items(): + for bundle in bundles.values(): if not self.has_alignment_impl and bundle.alignment_file.has_impl(): self.has_alignment_impl = True if not self.has_image_impl and bundle.image_file.has_impl(): From f0f615403ea0c830e2975ef777ce450a1af052da Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 17:04:05 -0800 Subject: [PATCH 11/35] FileBundleList: Make `cls.exclude_dirs` a `FrozenSet` --- ultratrace2/model/files/bundle.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ultratrace2/model/files/bundle.py b/ultratrace2/model/files/bundle.py index 56f8440..f096848 100644 --- a/ultratrace2/model/files/bundle.py +++ b/ultratrace2/model/files/bundle.py @@ -1,7 +1,7 @@ import logging import os -from typing import Dict, Sequence, Set +from typing import Dict, Sequence, FrozenSet from .impls import Sound, Alignment, ImageSet @@ -36,7 +36,7 @@ def __repr__(self): class FileBundleList: - exclude_dirs: Set[str] = set( + exclude_dirs: FrozenSet[str] = frozenset( [ ".git", "node_modules", From 6235875a8a2883597045712a2e801916c488b2ca Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 17:05:26 -0800 Subject: [PATCH 12/35] Update ultratrace2/model/files/bundle.py Co-Authored-By: Caleb --- 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 1de345d..9969854 100644 --- a/ultratrace2/model/files/bundle.py +++ b/ultratrace2/model/files/bundle.py @@ -69,7 +69,7 @@ def build_from_dir( ) -> "FileBundleList": # FIXME: implement `extra_exclude_dirs` as a command-line arg - exclude_dirs = cls.exclude_dirs.copy().union(extra_exclude_dirs) + exclude_dirs = cls.exclude_dirs.union(extra_exclude_dirs) bundles: Dict[str, FileBundle] = {} From 25aae65ae72254c3bb905721a9f8e535ba408bed Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 17:07:15 -0800 Subject: [PATCH 13/35] Update ultratrace2/model/files/bundle.py Co-Authored-By: Caleb --- 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 9969854..53f3f29 100644 --- a/ultratrace2/model/files/bundle.py +++ b/ultratrace2/model/files/bundle.py @@ -96,4 +96,4 @@ def build_from_dir( if not bundles[name].interpret(filepath): logger.warning(f"unrecognized filetype: {filepath}") - return FileBundleList(bundles) + return cls(bundles) From 67424ae1ccf5525ab5061856a954e7afe7450f4b Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 17:08:21 -0800 Subject: [PATCH 14/35] Update ultratrace2/model/project.py Co-Authored-By: Caleb --- ultratrace2/model/project.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ultratrace2/model/project.py b/ultratrace2/model/project.py index 24982b2..103f371 100644 --- a/ultratrace2/model/project.py +++ b/ultratrace2/model/project.py @@ -31,7 +31,7 @@ def load(cls, save_file) -> "Project": raise RuntimeError(str(e)) @classmethod - def get_by_path(cls, root_path) -> "Project": + def get_by_path(cls, root_path: str) -> "Project": root_path = os.path.realpath(os.path.abspath(root_path)) # absolute path if not os.path.exists(root_path) or not os.path.isdir(root_path): From 2ea02f04548e5f14eb4ce920df82c3f78d8aeccc Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 17:08:33 -0800 Subject: [PATCH 15/35] Update ultratrace2/model/project.py Co-Authored-By: Caleb --- ultratrace2/model/project.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ultratrace2/model/project.py b/ultratrace2/model/project.py index 103f371..c3e0297 100644 --- a/ultratrace2/model/project.py +++ b/ultratrace2/model/project.py @@ -20,7 +20,7 @@ def save(self): raise NotImplementedError() @classmethod - def load(cls, save_file) -> "Project": + def load(cls, save_file: str) -> "Project": try: with open(save_file, "rb") as fp: project = pickle.load(fp) From 2bee4995dc71c8dbbe669753009c034f2c2f0dd1 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 17:10:07 -0800 Subject: [PATCH 16/35] Project: Use more descriptive error messages in ::get_by_path() --- ultratrace2/model/project.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ultratrace2/model/project.py b/ultratrace2/model/project.py index 24982b2..7310da8 100644 --- a/ultratrace2/model/project.py +++ b/ultratrace2/model/project.py @@ -34,7 +34,12 @@ def load(cls, save_file) -> "Project": def get_by_path(cls, root_path) -> "Project": root_path = os.path.realpath(os.path.abspath(root_path)) # absolute path - if not os.path.exists(root_path) or not os.path.isdir(root_path): + if not os.path.exists(root_path): + raise ValueError( + f"cannot initialize project at {root_path}: directory does not exist" + ) + + if not os.path.isdir(root_path): raise ValueError( f"cannot initialize project at {root_path}: not a directory" ) From 935d9412aa691878807565e285bbba9c3d0deabd Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 17:11:13 -0800 Subject: [PATCH 17/35] Update ultratrace2/model/project.py Co-Authored-By: Caleb --- ultratrace2/model/project.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ultratrace2/model/project.py b/ultratrace2/model/project.py index b64de2b..733d3cf 100644 --- a/ultratrace2/model/project.py +++ b/ultratrace2/model/project.py @@ -44,7 +44,7 @@ def get_by_path(cls, root_path: str) -> "Project": f"cannot initialize project at {root_path}: not a directory" ) - save_dir = Project.get_save_dir(root_path) + save_dir = cls.get_save_dir(root_path) if not os.path.exists(save_dir): os.mkdir(save_dir, mode=0o755) From cc4b69620f142ad5ec1f405d6a47803ebf7ebdba Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 17:11:21 -0800 Subject: [PATCH 18/35] Update ultratrace2/model/project.py Co-Authored-By: Caleb --- ultratrace2/model/project.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ultratrace2/model/project.py b/ultratrace2/model/project.py index 733d3cf..b5f37fb 100644 --- a/ultratrace2/model/project.py +++ b/ultratrace2/model/project.py @@ -48,7 +48,7 @@ def get_by_path(cls, root_path: str) -> "Project": if not os.path.exists(save_dir): os.mkdir(save_dir, mode=0o755) - save_file = Project.get_save_file(root_path) + save_file = cls.get_save_file(root_path) try: return Project.load(save_file) except RuntimeError: From 5f2b6f81db99ec5b2880870420bbf68552730e0b Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 17:11:36 -0800 Subject: [PATCH 19/35] Update ultratrace2/model/project.py Co-Authored-By: Caleb --- ultratrace2/model/project.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ultratrace2/model/project.py b/ultratrace2/model/project.py index b5f37fb..39e3da3 100644 --- a/ultratrace2/model/project.py +++ b/ultratrace2/model/project.py @@ -50,7 +50,7 @@ def get_by_path(cls, root_path: str) -> "Project": save_file = cls.get_save_file(root_path) try: - return Project.load(save_file) + return cls.load(save_file) except RuntimeError: logger.info( f"Unable to find existing project at {root_path}, creating new one..." From dc181b5906fe33b82b61781da4d63cb7b188b6b0 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 17:11:48 -0800 Subject: [PATCH 20/35] Update ultratrace2/model/project.py Co-Authored-By: Caleb --- ultratrace2/model/project.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ultratrace2/model/project.py b/ultratrace2/model/project.py index 39e3da3..c95a419 100644 --- a/ultratrace2/model/project.py +++ b/ultratrace2/model/project.py @@ -58,7 +58,7 @@ def get_by_path(cls, root_path: str) -> "Project": traces = TraceList() file_bundles = FileBundleList.build_from_dir(root_path) - return Project(traces, file_bundles) + return cls(traces, file_bundles) @staticmethod def get_save_dir(path: str) -> str: From f7b7f7a43e8a45c105bd651a87ec4cded6ecedaf Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 17:23:51 -0800 Subject: [PATCH 21/35] App: be more explicit about allowed parameters to App constructor --- ultratrace2/__main__.py | 14 +++++++++++++- ultratrace2/app.py | 20 ++++++++++++-------- ultratrace2/tests/test_app.py | 27 ++++++++++++++------------- 3 files changed, 39 insertions(+), 22 deletions(-) diff --git a/ultratrace2/__main__.py b/ultratrace2/__main__.py index 94a9cf0..a5138f5 100644 --- a/ultratrace2/__main__.py +++ b/ultratrace2/__main__.py @@ -11,11 +11,22 @@ def main(): parser = argparse.ArgumentParser(prog="ultratrace") # noqa: E128 + parser.add_argument( + "--headless", + action="store_true", + default=False, + help="run ultratrace without a GUI interface", + ) parser.add_argument( "path", 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", @@ -56,7 +67,8 @@ def main(): args = parser.parse_args() - app = initialize_app(args) + app = initialize_app(headless=args.headless, path=args.path, theme=args.theme) + app.main() diff --git a/ultratrace2/app.py b/ultratrace2/app.py index 4192808..32fae47 100644 --- a/ultratrace2/app.py +++ b/ultratrace2/app.py @@ -1,4 +1,3 @@ -from argparse import Namespace from tkinter.filedialog import askdirectory as choose_dir from typing import Optional @@ -7,11 +6,13 @@ class App: - def __init__(self, args: Namespace): + def __init__( + self, + headless: bool = False, + path: Optional[str] = None, + theme: Optional[str] = None, + ): - headless = getattr(args, "headless", False) - - path: Optional[str] = getattr(args, "path", None) if path is None and not headless: path = choose_dir() if not path: @@ -20,7 +21,7 @@ def __init__(self, args: Namespace): self.project: Project = Project.get_by_path(path) if not headless: - self.gui = GUI(theme=args.theme) + self.gui = GUI(theme=theme) def main(self) -> None: pass @@ -30,7 +31,10 @@ def main(self) -> None: app: Optional[App] = None -def initialize_app(args: Namespace) -> App: +def initialize_app( + headless: bool = False, path: Optional[str] = None, theme: Optional[str] = None +) -> App: + global app - app = App(args) + app = App(headless=headless, path=path, theme=theme,) return app diff --git a/ultratrace2/tests/test_app.py b/ultratrace2/tests/test_app.py index 1002074..d6644c1 100644 --- a/ultratrace2/tests/test_app.py +++ b/ultratrace2/tests/test_app.py @@ -1,5 +1,5 @@ -from argparse import Namespace from pathlib import Path +from typing import Any, Dict import pytest @@ -8,29 +8,30 @@ @pytest.mark.parametrize( - "args,expected", + "kwargs,expected", [ - (Namespace(headless=True), ValueError), - (Namespace(headless=True, path="/path/to/nowhere"), ValueError), - (Namespace(headless=True, path="/dev/null"), ValueError), # not a directory - (Namespace(headless=True, path="/"), PermissionError), # not writeable + (dict(headless=True), ValueError), + (dict(headless=True, path="/path/to/nowhere"), ValueError), + (dict(headless=True, path="/dev/null"), ValueError), # not a directory + (dict(headless=True, path="/"), PermissionError), # not writeable ], ) -def test_initialize_app_invalid(args: Namespace, expected: Exception, tmpdir) -> None: +def test_initialize_app_invalid( + kwargs: Dict[str, Any], expected: Exception, tmpdir +) -> None: app_py.app = None # overwrite global object with pytest.raises(expected): - initialize_app(args) + initialize_app(**kwargs) -@pytest.mark.parametrize("args", [(Namespace(headless=True)),]) # noqa: E231 -def test_initialize_app_valid(args: Namespace, tmp_path: Path) -> None: +@pytest.mark.parametrize("kwargs", [(dict(headless=True)),]) # noqa: E231 +def test_initialize_app_valid(kwargs: Dict[str, Any], tmp_path: Path) -> None: # overwrite global object app_py.app = None # initialize an empty dir path = tmp_path / "ultratrace-test-app" path.mkdir() - # save that to the Namespace - args.path = path # initialize - app = initialize_app(args) + kwargs["path"] = str(path) + app = initialize_app(**kwargs) assert isinstance(app, App) From 0cb26688ef7283485403029ae82ba726e10c079d Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 17:42:21 -0800 Subject: [PATCH 22/35] App: change parameter name in one test --- ultratrace2/tests/test_app.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ultratrace2/tests/test_app.py b/ultratrace2/tests/test_app.py index d6644c1..6f4d02b 100644 --- a/ultratrace2/tests/test_app.py +++ b/ultratrace2/tests/test_app.py @@ -8,7 +8,7 @@ @pytest.mark.parametrize( - "kwargs,expected", + "kwargs,error", [ (dict(headless=True), ValueError), (dict(headless=True, path="/path/to/nowhere"), ValueError), @@ -17,10 +17,10 @@ ], ) def test_initialize_app_invalid( - kwargs: Dict[str, Any], expected: Exception, tmpdir + kwargs: Dict[str, Any], error: Exception, tmpdir ) -> None: app_py.app = None # overwrite global object - with pytest.raises(expected): + with pytest.raises(error): initialize_app(**kwargs) From c6b590a7fc514975967674d87e8cef5564f448b3 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 17:44:38 -0800 Subject: [PATCH 23/35] Project: add tests for ::load() NB: These tests only test for INVALID input. There are currently no tests for VALID input (since that behavior is not defined yet). --- ultratrace2/model/tests/test_project.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 ultratrace2/model/tests/test_project.py diff --git a/ultratrace2/model/tests/test_project.py b/ultratrace2/model/tests/test_project.py new file mode 100644 index 0000000..07de4b7 --- /dev/null +++ b/ultratrace2/model/tests/test_project.py @@ -0,0 +1,24 @@ +import pytest + +from ..project import Project + + +@pytest.mark.parametrize( + "save_file,error", + [ + ("", RuntimeError), + ("/path/to/nowhere", RuntimeError), + ("/dev/null", RuntimeError), # pickle cannot open + ("/etc/sudoers", RuntimeError), # not readable + ], +) +def test_load_project_invalid(save_file: str, error: Exception) -> None: + with pytest.raises(error): + Project.load(save_file) + + +# FIXME: not implemented, so can't test :/ +""" +def test_load_project_valid(save_file) -> None: + pass +""" From be611134a50a00a1c569297e18391c11cb79421d Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 17:47:23 -0800 Subject: [PATCH 24/35] nox: Also calculate test coverage when running pytest --- .gitignore | 1 + noxfile.py | 2 +- setup.cfg | 3 +++ setup.py | 1 + 4 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 setup.cfg diff --git a/.gitignore b/.gitignore index 2bba71b..daf1ea3 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,4 @@ __pycache__ *.pyc .mypy_cache .nox +.coverage diff --git a/noxfile.py b/noxfile.py index 6c6b986..c7dd2ae 100644 --- a/noxfile.py +++ b/noxfile.py @@ -29,4 +29,4 @@ def lint(session): @nox.session def tests(session): session.install(".[dev]") - session.run("pytest", "ultratrace2") \ No newline at end of file + session.run("pytest", "ultratrace2") diff --git a/setup.cfg b/setup.cfg new file mode 100644 index 0000000..6921bec --- /dev/null +++ b/setup.cfg @@ -0,0 +1,3 @@ +[tool:pytest] +addopts = + --cov=ultratrace2 diff --git a/setup.py b/setup.py index ab6d531..b0d6552 100644 --- a/setup.py +++ b/setup.py @@ -33,6 +33,7 @@ def get_requirement(line): "mypy", "pytest", "numpy-stubs @ git+https://github.com/numpy/numpy-stubs.git@master", + "pytest-cov", ] }, ) From a518eb84c166cb8d0d3f89e835db50a2678d41ac Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 18:20:47 -0800 Subject: [PATCH 25/35] FileBundle: Simplify logic of `has_*_impl` initialization --- ultratrace2/model/files/bundle.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/ultratrace2/model/files/bundle.py b/ultratrace2/model/files/bundle.py index 53f3f29..8709fa8 100644 --- a/ultratrace2/model/files/bundle.py +++ b/ultratrace2/model/files/bundle.py @@ -56,12 +56,9 @@ def __init__(self, bundles: Dict[str, FileBundle]): self.has_sound_impl: bool = False for bundle in bundles.values(): - if not self.has_alignment_impl and bundle.alignment_file.has_impl(): - self.has_alignment_impl = True - if not self.has_image_impl and bundle.image_file.has_impl(): - self.has_image_impl = True - if not self.has_sound_impl and bundle.sound_file.has_impl(): - self.has_sound_impl = True + self.has_alignment_impl |= bundle.alignment_file.has_impl() + self.has_image_impl |= bundle.image_file.has_impl() + self.has_sound_impl |= bundle.sound_file.has_impl() @classmethod def build_from_dir( From 318c618232f20aeb9d21f6dba06a092b13f57ce4 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 20:17:44 -0800 Subject: [PATCH 26/35] Files: simplify inheritance of file implementations Still unresolved: - How can we specify the desired resolution order (and still allow it be flexible / extensible)? --- ultratrace2/model/files/ADT.py | 89 -------------------- ultratrace2/model/files/adt.py | 59 +++++++++++++ ultratrace2/model/files/bundle.py | 67 ++++++++++----- ultratrace2/model/files/impls/__init__.py | 15 +++- ultratrace2/model/files/impls/alignment.py | 35 -------- ultratrace2/model/files/impls/dicom.py | 65 ++++++++++++++ ultratrace2/model/files/impls/flac.py | 8 ++ ultratrace2/model/files/impls/image.py | 71 ---------------- ultratrace2/model/files/impls/measurement.py | 10 +++ ultratrace2/model/files/impls/mp3.py | 8 ++ ultratrace2/model/files/impls/ogg.py | 8 ++ ultratrace2/model/files/impls/sound.py | 39 --------- ultratrace2/model/files/impls/textgrid.py | 7 ++ ultratrace2/model/files/impls/wav.py | 6 ++ 14 files changed, 227 insertions(+), 260 deletions(-) delete mode 100644 ultratrace2/model/files/ADT.py create mode 100644 ultratrace2/model/files/adt.py delete mode 100644 ultratrace2/model/files/impls/alignment.py create mode 100644 ultratrace2/model/files/impls/dicom.py create mode 100644 ultratrace2/model/files/impls/flac.py delete mode 100644 ultratrace2/model/files/impls/image.py create mode 100644 ultratrace2/model/files/impls/measurement.py create mode 100644 ultratrace2/model/files/impls/mp3.py create mode 100644 ultratrace2/model/files/impls/ogg.py delete mode 100644 ultratrace2/model/files/impls/sound.py create mode 100644 ultratrace2/model/files/impls/textgrid.py create mode 100644 ultratrace2/model/files/impls/wav.py diff --git a/ultratrace2/model/files/ADT.py b/ultratrace2/model/files/ADT.py deleted file mode 100644 index 285c119..0000000 --- a/ultratrace2/model/files/ADT.py +++ /dev/null @@ -1,89 +0,0 @@ -import logging -import os - -from abc import ABC, abstractmethod -from collections import OrderedDict -from magic import Magic # type: ignore -from typing import ClassVar, Dict, Optional, Sequence, Type - - -logger = logging.getLogger(__name__) - - -class FileLoadError(Exception): - pass - - -class TypedFile(ABC): - - preferred_impls = ClassVar[Sequence["TypedFileImpl"]] - - impls: Dict[Type["TypedFileImpl"], Optional["TypedFileImpl"]] - impl: Optional["TypedFileImpl"] - - def __new__(cls): - cls.impls = OrderedDict() - for impl_type in cls.preferred_impls: - cls.impls[impl_type] = None - return super().__new__(cls) - - def __init__(self): - self.impl = None - - def has_impl(self) -> bool: - return self.impl is not None - - def interpret(self, path: str) -> bool: - mimetype = Magic(mime=True).from_file(path) - _, extension = os.path.splitext(path.lower()) - recognized = False - for Impl in self.preferred_impls: - if Impl.recognizes(mimetype, extension): - recognized = True - if self.impl is not None: - logger.error( - f"cannot parse {path}: previous {type(Impl).__name__} was: {self.impl.path}, skipping..." - ) - continue - self.impl = Impl(path) - return True - return recognized - - def data(self): # FIXME: add signature - if self.impl is None: - raise ValueError("cannot load: no implementation found") - try: - self.impl.data() - except FileLoadError as e: - logger.error(f"unable to load {self.impl}: {e}") - self.impl = None # FIXME: is this sane behavior? - - def __repr__(self): - return f"{type(self).__name__}({self.impl})" - - -class TypedFileImpl(ABC): - - mimetypes: ClassVar[Sequence[str]] - extensions: ClassVar[Sequence[str]] - - def __init__(self, path: str): - self.path = path - self._data = None - - def data(self): # FIXME: add signature - # lazy load - if self._data is None: - self.load() - return self._data - - @abstractmethod - def load(self): # should throw FileLoadError if something went wrong - pass - - @classmethod - def recognizes(cls, mimetype: str, extension: str) -> bool: - return mimetype in cls.mimetypes or extension in cls.extensions - - def __repr__(self): - return f'{type(self).__name__}("{self.path}")' diff --git a/ultratrace2/model/files/adt.py b/ultratrace2/model/files/adt.py new file mode 100644 index 0000000..6a6ae5f --- /dev/null +++ b/ultratrace2/model/files/adt.py @@ -0,0 +1,59 @@ +import logging +import os + +from abc import ABC, abstractmethod +from magic import Magic # type: ignore +from typing import ClassVar, Sequence + + +logger = logging.getLogger(__name__) + + +class FileLoadError(Exception): + pass + + +class TypedFile(ABC): + + extensions: ClassVar[Sequence[str]] + mimetypes: ClassVar[Sequence[str]] + + _path: str + + @classmethod + def is_valid(cls, path: str) -> bool: + _, extension = os.path.splitext(path.lower()) + mimetype = Magic(mime=True).from_file(path) + return extension in cls.extensions and mimetype in cls.mimetypes + + @property + @abstractmethod + def path(self) -> str: + return self._path + + def __new__(cls, path: str): + try: + return cls.from_file(path) + except FileLoadError as e: + logger.error(e) + return None + + def __repr__(self): + return f"{type(self).__name__}({self.path})" + + @classmethod + @abstractmethod + def from_file(cls, path: str) -> "TypedFile": + pass + + +class AlignmentFile(TypedFile): + pass + + +class ImageSetFile(TypedFile): + pass + + +class SoundFile(TypedFile): + pass diff --git a/ultratrace2/model/files/bundle.py b/ultratrace2/model/files/bundle.py index 8709fa8..bbbf632 100644 --- a/ultratrace2/model/files/bundle.py +++ b/ultratrace2/model/files/bundle.py @@ -1,35 +1,42 @@ import logging import os -from typing import Dict, Sequence, FrozenSet +from typing import Dict, FrozenSet, Optional, Sequence, Type -from .impls import Sound, Alignment, ImageSet +from .adt import AlignmentFile, ImageSetFile, SoundFile, TypedFile logger = logging.getLogger(__name__) class FileBundle: - def __init__(self, name: str): + def __init__( + self, + name: str, + alignment_file: Optional[AlignmentFile] = None, + image_file: Optional[ImageSetFile] = None, + sound_file: Optional[SoundFile] = None, + ): self.name = name - self.alignment_file = Alignment() - self.image_file = ImageSet() - self.sound_file = Sound() - - def interpret(self, path: str): # FIXME: add signature - return ( - self.alignment_file.interpret(path) - or self.image_file.interpret(path) - or self.sound_file.interpret(path) - ) # noqa: E126 + self.alignment_file = alignment_file + self.image_file = image_file + self.sound_file = sound_file def has_impl(self) -> bool: - return ( - self.alignment_file.has_impl() - or self.image_file.has_impl() - or self.sound_file.has_impl() + return any( + f is not None + for f in [self.alignment_file, self.image_file, self.sound_file] ) + def set_alignment_file(self, alignment_file: AlignmentFile) -> None: + self.alignment_file = alignment_file + + def set_image_file(self, image_file: ImageSetFile) -> None: + self.image_file = image_file + + def set_sound_file(self, sound_file: SoundFile) -> None: + self.sound_file = sound_file + def __repr__(self): return f'Bundle("{self.name}",{self.alignment_file},{self.image_file},{self.sound_file})' @@ -56,9 +63,9 @@ def __init__(self, bundles: Dict[str, FileBundle]): self.has_sound_impl: bool = False for bundle in bundles.values(): - self.has_alignment_impl |= bundle.alignment_file.has_impl() - self.has_image_impl |= bundle.image_file.has_impl() - self.has_sound_impl |= bundle.sound_file.has_impl() + self.has_alignment_impl |= bundle.alignment_file is not None + self.has_image_impl |= bundle.image_file is not None + self.has_sound_impl |= bundle.sound_file is not None @classmethod def build_from_dir( @@ -87,10 +94,24 @@ def build_from_dir( ) continue + impl_cls = guess_file_type(filepath) + if impl_cls is None: + logger.warning(f"unrecognized filetype: {filepath}") + continue + if name not in bundles: - bundles[name] = FileBundle(name) + bundles[name] = FileBundle(filepath) - if not bundles[name].interpret(filepath): - logger.warning(f"unrecognized filetype: {filepath}") + file_impl = impl_cls(filepath) + if isinstance(file_impl, AlignmentFile): + bundles[name].set_alignment_file(file_impl) + elif isinstance(file_impl, ImageSetFile): + bundles[name].set_image_file(file_impl) + elif isinstance(file_impl, SoundFile): + bundles[name].set_sound_file(file_impl) return cls(bundles) + + +def guess_file_type(path: str) -> Optional[Type[TypedFile]]: + raise NotImplementedError() diff --git a/ultratrace2/model/files/impls/__init__.py b/ultratrace2/model/files/impls/__init__.py index 64f73eb..e1cf37e 100644 --- a/ultratrace2/model/files/impls/__init__.py +++ b/ultratrace2/model/files/impls/__init__.py @@ -1,3 +1,12 @@ -from .alignment import Alignment # noqa: F401 -from .image import ImageSet # noqa: F401 -from .sound import Sound # noqa: F401 +# alignment files +from .measurement import Measurement # noqa: F401 +from .textgrid import TextGrid # noqa: F401 + +# imageset files +from .dicom import DICOM # noqa: F401 + +# sound files +from .flac import FLAC # noqa: F401 +from .mp3 import MP3 # noqa: F401 +from .ogg import Ogg # noqa: F401 +from .wav import WAV # noqa: F401 diff --git a/ultratrace2/model/files/impls/alignment.py b/ultratrace2/model/files/impls/alignment.py deleted file mode 100644 index 5218dfd..0000000 --- a/ultratrace2/model/files/impls/alignment.py +++ /dev/null @@ -1,35 +0,0 @@ -from typing import ClassVar, Sequence - -from ..ADT import TypedFile, TypedFileImpl - - -class Alignment(TypedFile): - class TextGrid(TypedFileImpl): - - mimetypes = ["text/plain"] - extensions = [".textgrid"] - - def load(self): - raise NotImplementedError() - - @classmethod - def recognizes(cls, mimetype: str, extension: str) -> bool: - return mimetype in cls.mimetypes and extension in cls.extensions - - class Measurement(TypedFileImpl): - # FIXME: what is this? do we need to support it? - - mimetypes: ClassVar[Sequence[str]] = [] - extensions: ClassVar[Sequence[str]] = [] - - def load(self): - raise NotImplementedError() - - @classmethod - def recognizes(cls, mimetype: str, extension: str) -> bool: - return mimetype in cls.mimetypes and extension in cls.extensions - - preferred_impls = [TextGrid, Measurement] - - def __init__(self): - super().__init__() diff --git a/ultratrace2/model/files/impls/dicom.py b/ultratrace2/model/files/impls/dicom.py new file mode 100644 index 0000000..b23e2ac --- /dev/null +++ b/ultratrace2/model/files/impls/dicom.py @@ -0,0 +1,65 @@ +import numpy as np +import os +import PIL # type: ignore +import pydicom # type: ignore + +from tqdm import tqdm # type: ignore + +from ..adt import FileLoadError, TypedFile + + +class DICOM(TypedFile): + + mimetypes = ["application/dicom"] + extensions = [".dicom", ".dcm"] + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) # FIXME: make this more granular + self.png_path = f"{self.path}-frames" + + def load(self) -> None: + if os.path.exists(self.png_path): + return + + try: + dicom = pydicom.read_file(self.path) + except pydicom.errors.InvalidDicomError as e: + raise FileLoadError(str(e)) + + pixels: np.ndarray = dicom.pixel_array + + # check encoding, manipulate array if we need to + if len(pixels.shape) == 3: + is_greyscale = True + frames, rows, columns = pixels.shape + + elif len(pixels.shape) == 4: + # full-color RGB + is_greyscale = False + if pixels.shape[0] == 3: + # RGB-first + rgb, frames, rows, columns = pixels.shape + pixels.reshape((frames, rows, columns, rgb)) + elif pixels.shape[3] == 3: + # RGB-last + frames, rows, columns, rgb = pixels.shape + else: + raise FileLoadError( + "Invalid DICOM ({self.path}), unknown shape {pixels.shape}" + ) + + else: + raise FileLoadError( + "Invalid DICOM ({self.path}), unknown shape {pixels.shape}" + ) + + os.mkdir(self.png_path, mode=0o755) + for i in tqdm(range(frames), desc="converting to PNG"): + filename = os.path.join(self.png_path, f"{i:06}.png") + arr = pixels[i, :, :] if is_greyscale else pixels[i, :, :, :] + img = PIL.Image.fromarray(arr) + img.save(filename, format="PNG", compress_level=1) + + def convert_to_png(self, *args, **kwargs): + # FIXME: implement this function, signatures, etc. + print("converting") diff --git a/ultratrace2/model/files/impls/flac.py b/ultratrace2/model/files/impls/flac.py new file mode 100644 index 0000000..fd18158 --- /dev/null +++ b/ultratrace2/model/files/impls/flac.py @@ -0,0 +1,8 @@ +from typing import ClassVar, Sequence + +from ..adt import TypedFile + + +class FLAC(TypedFile): + mimetypes: ClassVar[Sequence[str]] = [] + extensions: ClassVar[Sequence[str]] = [] diff --git a/ultratrace2/model/files/impls/image.py b/ultratrace2/model/files/impls/image.py deleted file mode 100644 index 1979559..0000000 --- a/ultratrace2/model/files/impls/image.py +++ /dev/null @@ -1,71 +0,0 @@ -import numpy as np -import os -import PIL # type: ignore -import pydicom # type: ignore - -from tqdm import tqdm # type: ignore - -from ..ADT import FileLoadError, TypedFile, TypedFileImpl - - -class ImageSet(TypedFile): - class DICOM(TypedFileImpl): - - mimetypes = ["application/dicom"] - extensions = [".dicom", ".dcm"] - - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) # FIXME: make this more granular - self.png_path = f"{self.path}-frames" - - def load(self) -> None: - if os.path.exists(self.png_path): - return - - try: - dicom = pydicom.read_file(self.path) - except pydicom.errors.InvalidDicomError as e: - raise FileLoadError(str(e)) - - pixels: np.ndarray = dicom.pixel_array - - # check encoding, manipulate array if we need to - if len(pixels.shape) == 3: - is_greyscale = True - frames, rows, columns = pixels.shape - - elif len(pixels.shape) == 4: - # full-color RGB - is_greyscale = False - if pixels.shape[0] == 3: - # RGB-first - rgb, frames, rows, columns = pixels.shape - pixels.reshape((frames, rows, columns, rgb)) - elif pixels.shape[3] == 3: - # RGB-last - frames, rows, columns, rgb = pixels.shape - else: - raise FileLoadError( - "Invalid DICOM ({self.path}), unknown shape {pixels.shape}" - ) - - else: - raise FileLoadError( - "Invalid DICOM ({self.path}), unknown shape {pixels.shape}" - ) - - os.mkdir(self.png_path, mode=0o755) - for i in tqdm(range(frames), desc="converting to PNG"): - filename = os.path.join(self.png_path, f"{i:06}.png") - arr = pixels[i, :, :] if is_greyscale else pixels[i, :, :, :] - img = PIL.Image.fromarray(arr) - img.save(filename, format="PNG", compress_level=1) - - def convert_to_png(self, *args, **kwargs): - # FIXME: implement this function, signatures, etc. - print("converting") - - preferred_impls = [DICOM] - - def __init__(self): - super().__init__() diff --git a/ultratrace2/model/files/impls/measurement.py b/ultratrace2/model/files/impls/measurement.py new file mode 100644 index 0000000..0229efe --- /dev/null +++ b/ultratrace2/model/files/impls/measurement.py @@ -0,0 +1,10 @@ +from typing import ClassVar, Sequence + +from ..adt import AlignmentFile + + +class Measurement(AlignmentFile): + # FIXME: what is this? do we need to support it? + + mimetypes: ClassVar[Sequence[str]] = [] + extensions: ClassVar[Sequence[str]] = [] diff --git a/ultratrace2/model/files/impls/mp3.py b/ultratrace2/model/files/impls/mp3.py new file mode 100644 index 0000000..79ebf70 --- /dev/null +++ b/ultratrace2/model/files/impls/mp3.py @@ -0,0 +1,8 @@ +from typing import ClassVar, Sequence + +from ..adt import TypedFile + + +class MP3(TypedFile): + mimetypes: ClassVar[Sequence[str]] = [] + extensions: ClassVar[Sequence[str]] = [] diff --git a/ultratrace2/model/files/impls/ogg.py b/ultratrace2/model/files/impls/ogg.py new file mode 100644 index 0000000..17b4b3a --- /dev/null +++ b/ultratrace2/model/files/impls/ogg.py @@ -0,0 +1,8 @@ +from typing import ClassVar, Sequence + +from ..adt import TypedFile + + +class Ogg(TypedFile): + mimetypes: ClassVar[Sequence[str]] = [] + extensions: ClassVar[Sequence[str]] = [] diff --git a/ultratrace2/model/files/impls/sound.py b/ultratrace2/model/files/impls/sound.py deleted file mode 100644 index a448f86..0000000 --- a/ultratrace2/model/files/impls/sound.py +++ /dev/null @@ -1,39 +0,0 @@ -from typing import ClassVar, Sequence - -from ..ADT import TypedFile, TypedFileImpl - - -class Sound(TypedFile): - class WAV(TypedFileImpl): - - mimetypes = ["audio/x-wav", "audio/wav"] - extensions = ["wav"] - - def load(self): - raise NotImplementedError() - - class FLAC(TypedFileImpl): - mimetypes: ClassVar[Sequence[str]] = [] - extensions: ClassVar[Sequence[str]] = [] - - def load(self): - raise NotImplementedError() - - class Ogg(TypedFileImpl): - mimetypes: ClassVar[Sequence[str]] = [] - extensions: ClassVar[Sequence[str]] = [] - - def load(self): - raise NotImplementedError() - - class MP3(TypedFileImpl): - mimetypes: ClassVar[Sequence[str]] = [] - extensions: ClassVar[Sequence[str]] = [] - - def load(self): - raise NotImplementedError() - - preferred_impls = [WAV, FLAC, Ogg, MP3] - - def __init__(self): - super().__init__() diff --git a/ultratrace2/model/files/impls/textgrid.py b/ultratrace2/model/files/impls/textgrid.py new file mode 100644 index 0000000..3a8aa12 --- /dev/null +++ b/ultratrace2/model/files/impls/textgrid.py @@ -0,0 +1,7 @@ +from ..adt import AlignmentFile + + +class TextGrid(AlignmentFile): + + mimetypes = ["text/plain"] + extensions = [".textgrid"] diff --git a/ultratrace2/model/files/impls/wav.py b/ultratrace2/model/files/impls/wav.py new file mode 100644 index 0000000..7130e70 --- /dev/null +++ b/ultratrace2/model/files/impls/wav.py @@ -0,0 +1,6 @@ +from ..adt import TypedFile + + +class WAV(TypedFile): + mimetypes = ["audio/x-wav", "audio/wav"] + extensions = ["wav"] From 04c8af2a13e61f0b80a46755ce1696a86288d81c Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Wed, 18 Dec 2019 20:45:15 -0800 Subject: [PATCH 27/35] Remove `python-magic` dependency in favor of `mimetypes` Python already includes a standard library for working with mimetypes, so it's unclear why we were ever using `python-magic`. --- install.sh | 3 +-- requirements.txt | 1 - ultratrace2/model/files/adt.py | 4 ++-- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/install.sh b/install.sh index 71c53ed..7fc2ae7 100755 --- a/install.sh +++ b/install.sh @@ -1,12 +1,11 @@ #!/usr/bin/env bash if which brew &> /dev/null; then - brew install portaudio libmagic ffmpeg libav + brew install portaudio ffmpeg libav elif which apt-get &> /dev/null; then sudo apt-get update sudo apt-get install \ portaudio19-dev libportaudio2 \ - libmagic-dev \ ffmpeg \ libav-tools else diff --git a/requirements.txt b/requirements.txt index f5a67b6..8bc75c2 100644 --- a/requirements.txt +++ b/requirements.txt @@ -9,7 +9,6 @@ pydicom==1.3.0 pydub==0.23.1 pyparsing==2.4.5 python-dateutil==2.8.1 -python-magic==0.4.15 six==1.13.0 TextGrid==1.5 tqdm==4.40.2 diff --git a/ultratrace2/model/files/adt.py b/ultratrace2/model/files/adt.py index 6a6ae5f..07b0931 100644 --- a/ultratrace2/model/files/adt.py +++ b/ultratrace2/model/files/adt.py @@ -1,8 +1,8 @@ import logging +import mimetypes import os from abc import ABC, abstractmethod -from magic import Magic # type: ignore from typing import ClassVar, Sequence @@ -23,7 +23,7 @@ class TypedFile(ABC): @classmethod def is_valid(cls, path: str) -> bool: _, extension = os.path.splitext(path.lower()) - mimetype = Magic(mime=True).from_file(path) + mimetype, _ = mimetypes.guess_type(path) return extension in cls.extensions and mimetype in cls.mimetypes @property From cf17cff973426b050998db3254a24c29a9eda9e2 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Thu, 19 Dec 2019 08:37:23 -0800 Subject: [PATCH 28/35] Registry: Implement plugin-based architecture for loading files --- ultratrace2/model/files/__init__.py | 32 ++++++++ ultratrace2/model/files/adt.py | 59 -------------- ultratrace2/model/files/bundle.py | 61 ++++++++------ ultratrace2/model/files/impls/__init__.py | 12 --- ultratrace2/model/files/impls/dicom.py | 65 --------------- ultratrace2/model/files/impls/flac.py | 8 -- ultratrace2/model/files/impls/measurement.py | 10 --- ultratrace2/model/files/impls/mp3.py | 8 -- ultratrace2/model/files/impls/ogg.py | 8 -- ultratrace2/model/files/impls/textgrid.py | 7 -- ultratrace2/model/files/impls/wav.py | 6 -- ultratrace2/model/files/loaders/__init__.py | 12 +++ ultratrace2/model/files/loaders/base.py | 58 ++++++++++++++ ultratrace2/model/files/loaders/dicom.py | 79 +++++++++++++++++++ ultratrace2/model/files/loaders/flac.py | 7 ++ .../model/files/loaders/measurement.py | 9 +++ ultratrace2/model/files/loaders/mp3.py | 7 ++ ultratrace2/model/files/loaders/ogg.py | 7 ++ ultratrace2/model/files/loaders/textgrid.py | 7 ++ ultratrace2/model/files/loaders/wav.py | 7 ++ ultratrace2/model/files/registry.py | 67 ++++++++++++++++ ultratrace2/model/project.py | 2 +- ultratrace2/model/tests/test_project.py | 14 ++++ 23 files changed, 342 insertions(+), 210 deletions(-) delete mode 100644 ultratrace2/model/files/adt.py delete mode 100644 ultratrace2/model/files/impls/__init__.py delete mode 100644 ultratrace2/model/files/impls/dicom.py delete mode 100644 ultratrace2/model/files/impls/flac.py delete mode 100644 ultratrace2/model/files/impls/measurement.py delete mode 100644 ultratrace2/model/files/impls/mp3.py delete mode 100644 ultratrace2/model/files/impls/ogg.py delete mode 100644 ultratrace2/model/files/impls/textgrid.py delete mode 100644 ultratrace2/model/files/impls/wav.py create mode 100644 ultratrace2/model/files/loaders/__init__.py create mode 100644 ultratrace2/model/files/loaders/base.py create mode 100644 ultratrace2/model/files/loaders/dicom.py create mode 100644 ultratrace2/model/files/loaders/flac.py create mode 100644 ultratrace2/model/files/loaders/measurement.py create mode 100644 ultratrace2/model/files/loaders/mp3.py create mode 100644 ultratrace2/model/files/loaders/ogg.py create mode 100644 ultratrace2/model/files/loaders/textgrid.py create mode 100644 ultratrace2/model/files/loaders/wav.py create mode 100644 ultratrace2/model/files/registry.py diff --git a/ultratrace2/model/files/__init__.py b/ultratrace2/model/files/__init__.py index e69de29..b3b2552 100644 --- a/ultratrace2/model/files/__init__.py +++ b/ultratrace2/model/files/__init__.py @@ -0,0 +1,32 @@ +from .registry import register_loader_for_extensions_and_mime_types as __register + +from .loaders import DICOMLoader +from .loaders import FLACLoader +from .loaders import MeasurementLoader +from .loaders import MP3Loader +from .loaders import OggLoader +from .loaders import TextGridLoader +from .loaders import WAVLoader + + +__register( + [".dicom", ".dcm"], ["application/dicom"], DICOMLoader, +) +__register( + [], [], FLACLoader, +) +__register( + [], [], MeasurementLoader, +) +__register( + [], [], MP3Loader, +) +__register( + [], [], OggLoader, +) +__register( + [".textgrid"], ["text/plain"], TextGridLoader, +) +__register( + [".wav"], ["audio/x-wav", "audio/wav"], WAVLoader, +) diff --git a/ultratrace2/model/files/adt.py b/ultratrace2/model/files/adt.py deleted file mode 100644 index 07b0931..0000000 --- a/ultratrace2/model/files/adt.py +++ /dev/null @@ -1,59 +0,0 @@ -import logging -import mimetypes -import os - -from abc import ABC, abstractmethod -from typing import ClassVar, Sequence - - -logger = logging.getLogger(__name__) - - -class FileLoadError(Exception): - pass - - -class TypedFile(ABC): - - extensions: ClassVar[Sequence[str]] - mimetypes: ClassVar[Sequence[str]] - - _path: str - - @classmethod - def is_valid(cls, path: str) -> bool: - _, extension = os.path.splitext(path.lower()) - mimetype, _ = mimetypes.guess_type(path) - return extension in cls.extensions and mimetype in cls.mimetypes - - @property - @abstractmethod - def path(self) -> str: - return self._path - - def __new__(cls, path: str): - try: - return cls.from_file(path) - except FileLoadError as e: - logger.error(e) - return None - - def __repr__(self): - return f"{type(self).__name__}({self.path})" - - @classmethod - @abstractmethod - def from_file(cls, path: str) -> "TypedFile": - pass - - -class AlignmentFile(TypedFile): - pass - - -class ImageSetFile(TypedFile): - pass - - -class SoundFile(TypedFile): - pass diff --git a/ultratrace2/model/files/bundle.py b/ultratrace2/model/files/bundle.py index bbbf632..e2a5f96 100644 --- a/ultratrace2/model/files/bundle.py +++ b/ultratrace2/model/files/bundle.py @@ -3,7 +3,13 @@ from typing import Dict, FrozenSet, Optional, Sequence, Type -from .adt import AlignmentFile, ImageSetFile, SoundFile, TypedFile +from .loaders.base import ( + AlignmentFileLoader, + ImageSetFileLoader, + SoundFileLoader, + FileLoaderBase, +) +from .registry import get_loader_for logger = logging.getLogger(__name__) @@ -13,32 +19,38 @@ class FileBundle: def __init__( self, name: str, - alignment_file: Optional[AlignmentFile] = None, - image_file: Optional[ImageSetFile] = None, - sound_file: Optional[SoundFile] = None, + alignment_file: Optional[AlignmentFileLoader] = None, + image_set_file: Optional[ImageSetFileLoader] = None, + sound_file: Optional[SoundFileLoader] = None, ): self.name = name self.alignment_file = alignment_file - self.image_file = image_file + self.image_set_file = image_set_file self.sound_file = sound_file def has_impl(self) -> bool: return any( f is not None - for f in [self.alignment_file, self.image_file, self.sound_file] + for f in [self.alignment_file, self.image_set_file, self.sound_file] ) - def set_alignment_file(self, alignment_file: AlignmentFile) -> None: + 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 set_image_file(self, image_file: ImageSetFile) -> None: - self.image_file = image_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 set_sound_file(self, sound_file: SoundFile) -> None: + def set_sound_file(self, sound_file: SoundFileLoader) -> None: + if self.sound_file is not None: + logger.warning("Overwriting existing sound file") self.sound_file = sound_file def __repr__(self): - return f'Bundle("{self.name}",{self.alignment_file},{self.image_file},{self.sound_file})' + return f'Bundle("{self.name}",{self.alignment_file},{self.image_set_file},{self.sound_file})' class FileBundleList: @@ -59,12 +71,12 @@ def __init__(self, bundles: Dict[str, FileBundle]): self.bundles: Dict[str, FileBundle] = bundles self.has_alignment_impl: bool = False - self.has_image_impl: bool = False + self.has_image_set_impl: bool = False self.has_sound_impl: bool = False for bundle in bundles.values(): self.has_alignment_impl |= bundle.alignment_file is not None - self.has_image_impl |= bundle.image_file is not None + self.has_image_set_impl |= bundle.image_set_file is not None self.has_sound_impl |= bundle.sound_file is not None @classmethod @@ -94,24 +106,21 @@ def build_from_dir( ) continue - impl_cls = guess_file_type(filepath) - if impl_cls is None: + file_loader: Optional[Type[FileLoaderBase]] = get_loader_for(filepath) + if file_loader is None: logger.warning(f"unrecognized filetype: {filepath}") continue if name not in bundles: bundles[name] = FileBundle(filepath) - file_impl = impl_cls(filepath) - if isinstance(file_impl, AlignmentFile): - bundles[name].set_alignment_file(file_impl) - elif isinstance(file_impl, ImageSetFile): - bundles[name].set_image_file(file_impl) - elif isinstance(file_impl, SoundFile): - bundles[name].set_sound_file(file_impl) + loaded_file = file_loader(filepath) + if loaded_file is not None: + if isinstance(loaded_file, AlignmentFileLoader): + bundles[name].set_alignment_file(loaded_file) + elif isinstance(loaded_file, ImageSetFileLoader): + bundles[name].set_image_set_file(loaded_file) + elif isinstance(loaded_file, SoundFileLoader): + bundles[name].set_sound_file(loaded_file) return cls(bundles) - - -def guess_file_type(path: str) -> Optional[Type[TypedFile]]: - raise NotImplementedError() diff --git a/ultratrace2/model/files/impls/__init__.py b/ultratrace2/model/files/impls/__init__.py deleted file mode 100644 index e1cf37e..0000000 --- a/ultratrace2/model/files/impls/__init__.py +++ /dev/null @@ -1,12 +0,0 @@ -# alignment files -from .measurement import Measurement # noqa: F401 -from .textgrid import TextGrid # noqa: F401 - -# imageset files -from .dicom import DICOM # noqa: F401 - -# sound files -from .flac import FLAC # noqa: F401 -from .mp3 import MP3 # noqa: F401 -from .ogg import Ogg # noqa: F401 -from .wav import WAV # noqa: F401 diff --git a/ultratrace2/model/files/impls/dicom.py b/ultratrace2/model/files/impls/dicom.py deleted file mode 100644 index b23e2ac..0000000 --- a/ultratrace2/model/files/impls/dicom.py +++ /dev/null @@ -1,65 +0,0 @@ -import numpy as np -import os -import PIL # type: ignore -import pydicom # type: ignore - -from tqdm import tqdm # type: ignore - -from ..adt import FileLoadError, TypedFile - - -class DICOM(TypedFile): - - mimetypes = ["application/dicom"] - extensions = [".dicom", ".dcm"] - - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) # FIXME: make this more granular - self.png_path = f"{self.path}-frames" - - def load(self) -> None: - if os.path.exists(self.png_path): - return - - try: - dicom = pydicom.read_file(self.path) - except pydicom.errors.InvalidDicomError as e: - raise FileLoadError(str(e)) - - pixels: np.ndarray = dicom.pixel_array - - # check encoding, manipulate array if we need to - if len(pixels.shape) == 3: - is_greyscale = True - frames, rows, columns = pixels.shape - - elif len(pixels.shape) == 4: - # full-color RGB - is_greyscale = False - if pixels.shape[0] == 3: - # RGB-first - rgb, frames, rows, columns = pixels.shape - pixels.reshape((frames, rows, columns, rgb)) - elif pixels.shape[3] == 3: - # RGB-last - frames, rows, columns, rgb = pixels.shape - else: - raise FileLoadError( - "Invalid DICOM ({self.path}), unknown shape {pixels.shape}" - ) - - else: - raise FileLoadError( - "Invalid DICOM ({self.path}), unknown shape {pixels.shape}" - ) - - os.mkdir(self.png_path, mode=0o755) - for i in tqdm(range(frames), desc="converting to PNG"): - filename = os.path.join(self.png_path, f"{i:06}.png") - arr = pixels[i, :, :] if is_greyscale else pixels[i, :, :, :] - img = PIL.Image.fromarray(arr) - img.save(filename, format="PNG", compress_level=1) - - def convert_to_png(self, *args, **kwargs): - # FIXME: implement this function, signatures, etc. - print("converting") diff --git a/ultratrace2/model/files/impls/flac.py b/ultratrace2/model/files/impls/flac.py deleted file mode 100644 index fd18158..0000000 --- a/ultratrace2/model/files/impls/flac.py +++ /dev/null @@ -1,8 +0,0 @@ -from typing import ClassVar, Sequence - -from ..adt import TypedFile - - -class FLAC(TypedFile): - mimetypes: ClassVar[Sequence[str]] = [] - extensions: ClassVar[Sequence[str]] = [] diff --git a/ultratrace2/model/files/impls/measurement.py b/ultratrace2/model/files/impls/measurement.py deleted file mode 100644 index 0229efe..0000000 --- a/ultratrace2/model/files/impls/measurement.py +++ /dev/null @@ -1,10 +0,0 @@ -from typing import ClassVar, Sequence - -from ..adt import AlignmentFile - - -class Measurement(AlignmentFile): - # FIXME: what is this? do we need to support it? - - mimetypes: ClassVar[Sequence[str]] = [] - extensions: ClassVar[Sequence[str]] = [] diff --git a/ultratrace2/model/files/impls/mp3.py b/ultratrace2/model/files/impls/mp3.py deleted file mode 100644 index 79ebf70..0000000 --- a/ultratrace2/model/files/impls/mp3.py +++ /dev/null @@ -1,8 +0,0 @@ -from typing import ClassVar, Sequence - -from ..adt import TypedFile - - -class MP3(TypedFile): - mimetypes: ClassVar[Sequence[str]] = [] - extensions: ClassVar[Sequence[str]] = [] diff --git a/ultratrace2/model/files/impls/ogg.py b/ultratrace2/model/files/impls/ogg.py deleted file mode 100644 index 17b4b3a..0000000 --- a/ultratrace2/model/files/impls/ogg.py +++ /dev/null @@ -1,8 +0,0 @@ -from typing import ClassVar, Sequence - -from ..adt import TypedFile - - -class Ogg(TypedFile): - mimetypes: ClassVar[Sequence[str]] = [] - extensions: ClassVar[Sequence[str]] = [] diff --git a/ultratrace2/model/files/impls/textgrid.py b/ultratrace2/model/files/impls/textgrid.py deleted file mode 100644 index 3a8aa12..0000000 --- a/ultratrace2/model/files/impls/textgrid.py +++ /dev/null @@ -1,7 +0,0 @@ -from ..adt import AlignmentFile - - -class TextGrid(AlignmentFile): - - mimetypes = ["text/plain"] - extensions = [".textgrid"] diff --git a/ultratrace2/model/files/impls/wav.py b/ultratrace2/model/files/impls/wav.py deleted file mode 100644 index 7130e70..0000000 --- a/ultratrace2/model/files/impls/wav.py +++ /dev/null @@ -1,6 +0,0 @@ -from ..adt import TypedFile - - -class WAV(TypedFile): - mimetypes = ["audio/x-wav", "audio/wav"] - extensions = ["wav"] diff --git a/ultratrace2/model/files/loaders/__init__.py b/ultratrace2/model/files/loaders/__init__.py new file mode 100644 index 0000000..c27a3dd --- /dev/null +++ b/ultratrace2/model/files/loaders/__init__.py @@ -0,0 +1,12 @@ +# alignment files +from .measurement import MeasurementLoader # noqa: F401 +from .textgrid import TextGridLoader # noqa: F401 + +# imageset files +from .dicom import DICOMLoader # noqa: F401 + +# sound files +from .flac import FLACLoader # noqa: F401 +from .mp3 import MP3Loader # noqa: F401 +from .ogg import OggLoader # noqa: F401 +from .wav import WAVLoader # noqa: F401 diff --git a/ultratrace2/model/files/loaders/base.py b/ultratrace2/model/files/loaders/base.py new file mode 100644 index 0000000..ccc8585 --- /dev/null +++ b/ultratrace2/model/files/loaders/base.py @@ -0,0 +1,58 @@ +import logging + +from abc import ABC, abstractmethod +from PIL import Image # type: ignore + + +logger = logging.getLogger(__name__) + + +class FileLoadError(Exception): + pass + + +class FileLoaderBase(ABC): + def __new__(cls, path: str): + try: + return cls.from_file(path) + except FileLoadError as e: + logger.error(e) + return None + + def __init__(self, path: str): + self.path = path + + def __repr__(self): + return f"{type(self).__name__}({self.path})" + + @classmethod + @abstractmethod + def from_file(cls, path: str) -> "FileLoaderBase": + # NB: If this concrete method fails to load the data at the given path, then + # it should throw a `FileLoadError`. + pass + + +class AlignmentFileLoader(FileLoaderBase): + pass + + +class ImageSetFileLoader(FileLoaderBase): + @abstractmethod + def __len__(self) -> int: + """ImageSets should have some notion of their length. + + For example, for DICOM files, this is equal to the number of frames. This + number can then be used to "slice up" any accompanying Alignment or Sound + files. + """ + pass + + @abstractmethod + def get_frame(self, i: int) -> Image.Image: + """ImageSets should support random access of frames.""" + pass + + +class SoundFileLoader(FileLoaderBase): + pass diff --git a/ultratrace2/model/files/loaders/dicom.py b/ultratrace2/model/files/loaders/dicom.py new file mode 100644 index 0000000..498d76a --- /dev/null +++ b/ultratrace2/model/files/loaders/dicom.py @@ -0,0 +1,79 @@ +from PIL import Image # type: ignore + +import numpy as np +import os +import pydicom # type: ignore + +from .base import FileLoadError, ImageSetFileLoader + + +class DICOMLoader(ImageSetFileLoader): + def __init__(self, path: str, pixels: np.ndarray): + """Construct the DICOMLoader from a pixel array. + + The `shape` of the pixel array should be `(n_frames, n_rows, n_columns)` for greyscale + images and `(n_frames, n_rows, n_columns, rgb_data)` for full-color images.""" + super().__init__(path) + self.pixels = pixels + # FIXME: these should be in the `.ultratrace/` dir + self.png_dir = f"{self.path}-frames" + if not os.path.exists(self.png_dir): + os.mkdir(self.png_dir, mode=0o755) + + def is_greyscale(self) -> bool: + return len(self.pixels) == 3 + + def __len__(self) -> int: + return self.pixels.shape[0] + + def get_png_filepath_for_frame(self, i: int) -> str: + return os.path.join(self.png_dir, f"{i:06}.png") + + def get_frame(self, i: int) -> Image.Image: + png_filepath = self.get_png_filepath_for_frame(i) + if os.path.exists(png_filepath): + return Image.open(png_filepath) + else: + arr = ( + self.pixels[i, :, :] if self.is_greyscale() else self.pixels[i, :, :, :] + ) + img = Image.fromarray(arr) + img.save(png_filepath, format="PNG", compress_level=1) + return img + + @classmethod + def from_file(cls, path: str) -> "DICOMLoader": + + if not os.path.exists(path): + raise FileLoadError("Cannot load from path: '{path}': cannot find") + + try: + dicom = pydicom.read_file(path) + except pydicom.errors.InvalidDicomError as e: + raise FileLoadError(str(e)) + + pixels: np.ndarray = dicom.pixel_array + + # check encoding, manipulate array if we need to + if len(pixels.shape) == 3: + n_frames, n_rows, n_columns = pixels.shape + return cls(path, pixels) + + elif len(pixels.shape) == 4: + # full-color RGB + if pixels.shape[0] == 3: + # RGB-first + rgb, n_frames, n_rows, n_columns = pixels.shape + pixels.reshape((n_frames, n_rows, n_columns, rgb)) + return cls(path, pixels) + + elif pixels.shape[3] == 3: + # RGB-last + n_frames, n_rows, n_columns, rgb = pixels.shape + return cls(path, pixels) + + raise FileLoadError("Invalid DICOM ({path}), unknown shape {pixels.shape}") + + def convert_to_png(self, *args, **kwargs): + # FIXME: implement this as a helper function + raise NotImplementedError() diff --git a/ultratrace2/model/files/loaders/flac.py b/ultratrace2/model/files/loaders/flac.py new file mode 100644 index 0000000..fdd9b75 --- /dev/null +++ b/ultratrace2/model/files/loaders/flac.py @@ -0,0 +1,7 @@ +from .base import SoundFileLoader + + +class FLACLoader(SoundFileLoader): + @classmethod + def from_file(cls, path: str) -> "FLACLoader": + raise NotImplementedError() diff --git a/ultratrace2/model/files/loaders/measurement.py b/ultratrace2/model/files/loaders/measurement.py new file mode 100644 index 0000000..a586179 --- /dev/null +++ b/ultratrace2/model/files/loaders/measurement.py @@ -0,0 +1,9 @@ +from .base import AlignmentFileLoader + + +class MeasurementLoader(AlignmentFileLoader): + # FIXME: what is this? do we need to support it? + + @classmethod + def from_file(cls, path: str) -> "MeasurementLoader": + raise NotImplementedError() diff --git a/ultratrace2/model/files/loaders/mp3.py b/ultratrace2/model/files/loaders/mp3.py new file mode 100644 index 0000000..b183616 --- /dev/null +++ b/ultratrace2/model/files/loaders/mp3.py @@ -0,0 +1,7 @@ +from .base import SoundFileLoader + + +class MP3Loader(SoundFileLoader): + @classmethod + def from_file(cls, path: str) -> "MP3Loader": + raise NotImplementedError() diff --git a/ultratrace2/model/files/loaders/ogg.py b/ultratrace2/model/files/loaders/ogg.py new file mode 100644 index 0000000..d70794c --- /dev/null +++ b/ultratrace2/model/files/loaders/ogg.py @@ -0,0 +1,7 @@ +from .base import SoundFileLoader + + +class OggLoader(SoundFileLoader): + @classmethod + def from_file(cls, path: str) -> "OggLoader": + raise NotImplementedError() diff --git a/ultratrace2/model/files/loaders/textgrid.py b/ultratrace2/model/files/loaders/textgrid.py new file mode 100644 index 0000000..b93a6f3 --- /dev/null +++ b/ultratrace2/model/files/loaders/textgrid.py @@ -0,0 +1,7 @@ +from .base import AlignmentFileLoader + + +class TextGridLoader(AlignmentFileLoader): + @classmethod + def from_file(cls, path: str) -> "TextGridLoader": + raise NotImplementedError() diff --git a/ultratrace2/model/files/loaders/wav.py b/ultratrace2/model/files/loaders/wav.py new file mode 100644 index 0000000..4cb593e --- /dev/null +++ b/ultratrace2/model/files/loaders/wav.py @@ -0,0 +1,7 @@ +from .base import SoundFileLoader + + +class WAVLoader(SoundFileLoader): + @classmethod + def from_file(cls, path: str) -> "WAVLoader": + raise NotImplementedError() diff --git a/ultratrace2/model/files/registry.py b/ultratrace2/model/files/registry.py new file mode 100644 index 0000000..f8d3da0 --- /dev/null +++ b/ultratrace2/model/files/registry.py @@ -0,0 +1,67 @@ +import mimetypes +import os + +from collections import defaultdict +from typing import DefaultDict, Optional, Sequence, Set, Type + +from .loaders.base import FileLoaderBase + + +# 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 +) + + +def register_loader_for_extensions_and_mime_types( + extensions: Sequence[str], + mime_types: Sequence[str], + loader_cls: Type[FileLoaderBase], +) -> None: + """Register a loader which recognizes a file in the format indicated by the given + extensions and MIME types. + + Parameters: + extensions: A set of file extension, e.g. [".wav"], indicating the file format + mime_types: A set of MIME types, e.g. ["audio/x-wav", "audio/wav"], also indicating the file format + loader_cls: A file loader which knows how to load files with the given file extensions and MIME types + """ + + global __extension_to_loaders_map + global __mime_type_to_loaders_map + + for extension in extensions: + __extension_to_loaders_map[extension].add(loader_cls) + + for mime_type in mime_types: + __mime_type_to_loaders_map[mime_type].add(loader_cls) + + +def get_loader_for(path: str) -> Optional[Type[FileLoaderBase]]: + + global __extension_to_loaders_map + global __mime_type_to_loaders_map + + _, extension = os.path.splitext(path.lower()) + mime_type, _ = mimetypes.guess_type(path) + if mime_type is None: + # 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] + + # 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: + return None + elif len(loader_clses) == 1: + return loader_clses.pop() + else: + raise NotImplementedError( + f"Found multiple Loaders for path '{path}': {','.join(map(str, loader_clses))}" + ) diff --git a/ultratrace2/model/project.py b/ultratrace2/model/project.py index c95a419..9dd59bd 100644 --- a/ultratrace2/model/project.py +++ b/ultratrace2/model/project.py @@ -82,7 +82,7 @@ def has_alignment_impl(self) -> bool: return self.files.has_alignment_impl def has_image_impl(self) -> bool: - return self.files.has_image_impl + return self.files.has_image_set_impl def has_sound_impl(self) -> bool: return self.files.has_sound_impl diff --git a/ultratrace2/model/tests/test_project.py b/ultratrace2/model/tests/test_project.py index 07de4b7..d46c396 100644 --- a/ultratrace2/model/tests/test_project.py +++ b/ultratrace2/model/tests/test_project.py @@ -1,6 +1,8 @@ import pytest from ..project import Project +from ..trace import TraceList +from ..files.bundle import FileBundle, FileBundleList @pytest.mark.parametrize( @@ -22,3 +24,15 @@ def test_load_project_invalid(save_file: str, error: Exception) -> None: def test_load_project_valid(save_file) -> None: pass """ + + +@pytest.mark.parametrize( + "traces,files", + [ + (TraceList(), FileBundleList({})), + (TraceList(), FileBundleList({"x": FileBundle("x")})), + ], +) +def test_init_project(traces: TraceList, files: FileBundleList) -> None: + p = Project(traces, files) + assert isinstance(p, Project) From a452e985a466b4360da07c0cb402694d9b194323 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Thu, 19 Dec 2019 17:53:55 -0800 Subject: [PATCH 29/35] FileLoaderBase: constrain ::from_file return type See https://github.com/SwatPhonLab/UltraTrace/pull/109#discussion_r360178018 for more context. --- ultratrace2/model/files/loaders/base.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ultratrace2/model/files/loaders/base.py b/ultratrace2/model/files/loaders/base.py index ccc8585..b31b0b9 100644 --- a/ultratrace2/model/files/loaders/base.py +++ b/ultratrace2/model/files/loaders/base.py @@ -2,6 +2,7 @@ from abc import ABC, abstractmethod from PIL import Image # type: ignore +from typing import Type, TypeVar logger = logging.getLogger(__name__) @@ -11,6 +12,9 @@ class FileLoadError(Exception): pass +Self = TypeVar("Self", bound="FileLoaderBase") + + class FileLoaderBase(ABC): def __new__(cls, path: str): try: @@ -27,7 +31,7 @@ def __repr__(self): @classmethod @abstractmethod - def from_file(cls, path: str) -> "FileLoaderBase": + 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`. pass From e38b084c21a3c3d8bf440aa02bc3d1a66f60609c Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Thu, 19 Dec 2019 17:56:42 -0800 Subject: [PATCH 30/35] Update ultratrace2/model/files/loaders/base.py Co-Authored-By: Caleb Ho --- ultratrace2/model/files/loaders/base.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ultratrace2/model/files/loaders/base.py b/ultratrace2/model/files/loaders/base.py index b31b0b9..cba6c3e 100644 --- a/ultratrace2/model/files/loaders/base.py +++ b/ultratrace2/model/files/loaders/base.py @@ -50,7 +50,6 @@ def __len__(self) -> int: number can then be used to "slice up" any accompanying Alignment or Sound files. """ - pass @abstractmethod def get_frame(self, i: int) -> Image.Image: From 336f414e23916aced760b5bdfbbfe9bd2588ede8 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Thu, 19 Dec 2019 17:59:59 -0800 Subject: [PATCH 31/35] DICOMLoader: raise FileLoadError from InvalidDicomError --- ultratrace2/model/files/loaders/dicom.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ultratrace2/model/files/loaders/dicom.py b/ultratrace2/model/files/loaders/dicom.py index 498d76a..86670d9 100644 --- a/ultratrace2/model/files/loaders/dicom.py +++ b/ultratrace2/model/files/loaders/dicom.py @@ -50,7 +50,9 @@ def from_file(cls, path: str) -> "DICOMLoader": try: dicom = pydicom.read_file(path) except pydicom.errors.InvalidDicomError as e: - raise FileLoadError(str(e)) + raise FileLoadError( + "Invalid DICOM ({path}), unable to read: {str(e)}" + ) from e pixels: np.ndarray = dicom.pixel_array From fa457a1c339776b9af46ad4dfa41fe4c74160a55 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Thu, 19 Dec 2019 18:01:29 -0800 Subject: [PATCH 32/35] Registry: remove unnecessary `global` statements Apparently, `global` is only necessary for assignment, and these functions only need to access the value. --- ultratrace2/model/files/registry.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/ultratrace2/model/files/registry.py b/ultratrace2/model/files/registry.py index f8d3da0..4766fb3 100644 --- a/ultratrace2/model/files/registry.py +++ b/ultratrace2/model/files/registry.py @@ -30,9 +30,6 @@ 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 """ - global __extension_to_loaders_map - global __mime_type_to_loaders_map - for extension in extensions: __extension_to_loaders_map[extension].add(loader_cls) @@ -42,9 +39,6 @@ def register_loader_for_extensions_and_mime_types( def get_loader_for(path: str) -> Optional[Type[FileLoaderBase]]: - global __extension_to_loaders_map - global __mime_type_to_loaders_map - _, extension = os.path.splitext(path.lower()) mime_type, _ = mimetypes.guess_type(path) if mime_type is None: From 74506114323aedf9873621b2826a19c2a7068dd0 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Thu, 19 Dec 2019 18:03:46 -0800 Subject: [PATCH 33/35] Project: Don't funnel ::load() errors into RuntimeErrors Instead, let the exceptions propagate up to the caller. For one thing, this allows us to write more specific tests :^) --- ultratrace2/model/project.py | 15 ++++++--------- ultratrace2/model/tests/test_project.py | 8 ++++---- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/ultratrace2/model/project.py b/ultratrace2/model/project.py index 9dd59bd..9345ab9 100644 --- a/ultratrace2/model/project.py +++ b/ultratrace2/model/project.py @@ -21,14 +21,10 @@ def save(self): @classmethod def load(cls, save_file: str) -> "Project": - try: - with open(save_file, "rb") as fp: - project = pickle.load(fp) - assert isinstance(project, Project) - return project - except Exception as e: - logger.error(e) - raise RuntimeError(str(e)) + with open(save_file, "rb") as fp: + project = pickle.load(fp) + assert isinstance(project, Project) + return project @classmethod def get_by_path(cls, root_path: str) -> "Project": @@ -51,7 +47,8 @@ def get_by_path(cls, root_path: str) -> "Project": save_file = cls.get_save_file(root_path) try: return cls.load(save_file) - except RuntimeError: + except Exception as e: + logger.warning(e) logger.info( f"Unable to find existing project at {root_path}, creating new one..." ) diff --git a/ultratrace2/model/tests/test_project.py b/ultratrace2/model/tests/test_project.py index d46c396..38c7f7e 100644 --- a/ultratrace2/model/tests/test_project.py +++ b/ultratrace2/model/tests/test_project.py @@ -8,10 +8,10 @@ @pytest.mark.parametrize( "save_file,error", [ - ("", RuntimeError), - ("/path/to/nowhere", RuntimeError), - ("/dev/null", RuntimeError), # pickle cannot open - ("/etc/sudoers", RuntimeError), # not readable + ("", FileNotFoundError), + ("/path/to/nowhere", FileNotFoundError), + ("/dev/null", EOFError), # pickle cannot open + ("/etc/sudoers", PermissionError), # not readable ], ) def test_load_project_invalid(save_file: str, error: Exception) -> None: From e6895ba21bceb9add3a47056b3e1d1304ec1d8f3 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Thu, 19 Dec 2019 18:26:01 -0800 Subject: [PATCH 34/35] FileBundle: Pass responsibility for handling FileLoadError to caller --- ultratrace2/model/files/bundle.py | 20 ++++++++++++-------- ultratrace2/model/files/loaders/base.py | 6 ------ 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/ultratrace2/model/files/bundle.py b/ultratrace2/model/files/bundle.py index e2a5f96..10216bf 100644 --- a/ultratrace2/model/files/bundle.py +++ b/ultratrace2/model/files/bundle.py @@ -8,6 +8,7 @@ ImageSetFileLoader, SoundFileLoader, FileLoaderBase, + FileLoadError, ) from .registry import get_loader_for @@ -114,13 +115,16 @@ def build_from_dir( if name not in bundles: bundles[name] = FileBundle(filepath) - loaded_file = file_loader(filepath) - if loaded_file is not None: - if isinstance(loaded_file, AlignmentFileLoader): - bundles[name].set_alignment_file(loaded_file) - elif isinstance(loaded_file, ImageSetFileLoader): - bundles[name].set_image_set_file(loaded_file) - elif isinstance(loaded_file, SoundFileLoader): - bundles[name].set_sound_file(loaded_file) + try: + loaded_file = file_loader.from_file(filepath) + if loaded_file is not None: + if isinstance(loaded_file, AlignmentFileLoader): + bundles[name].set_alignment_file(loaded_file) + elif isinstance(loaded_file, ImageSetFileLoader): + bundles[name].set_image_set_file(loaded_file) + elif isinstance(loaded_file, SoundFileLoader): + bundles[name].set_sound_file(loaded_file) + except FileLoadError as e: + logger.error(e) return cls(bundles) diff --git a/ultratrace2/model/files/loaders/base.py b/ultratrace2/model/files/loaders/base.py index cba6c3e..b8dd3b6 100644 --- a/ultratrace2/model/files/loaders/base.py +++ b/ultratrace2/model/files/loaders/base.py @@ -16,12 +16,6 @@ class FileLoadError(Exception): class FileLoaderBase(ABC): - def __new__(cls, path: str): - try: - return cls.from_file(path) - except FileLoadError as e: - logger.error(e) - return None def __init__(self, path: str): self.path = path From 2669b54b699228ac51bc1c267fffe07526de2116 Mon Sep 17 00:00:00 2001 From: Kevin Murphy Date: Thu, 19 Dec 2019 18:47:20 -0800 Subject: [PATCH 35/35] FileLoaderBase: Require `path: str` abstract attribute --- ultratrace2/model/files/loaders/base.py | 17 ++++++++++++----- ultratrace2/model/files/loaders/dicom.py | 8 +++++++- ultratrace2/model/files/loaders/flac.py | 9 +++++++++ ultratrace2/model/files/loaders/measurement.py | 9 +++++++++ ultratrace2/model/files/loaders/mp3.py | 9 +++++++++ ultratrace2/model/files/loaders/ogg.py | 9 +++++++++ ultratrace2/model/files/loaders/textgrid.py | 9 +++++++++ ultratrace2/model/files/loaders/wav.py | 9 +++++++++ 8 files changed, 73 insertions(+), 6 deletions(-) diff --git a/ultratrace2/model/files/loaders/base.py b/ultratrace2/model/files/loaders/base.py index b8dd3b6..b98ffb7 100644 --- a/ultratrace2/model/files/loaders/base.py +++ b/ultratrace2/model/files/loaders/base.py @@ -16,9 +16,15 @@ class FileLoadError(Exception): class FileLoaderBase(ABC): + @abstractmethod + def get_path(self) -> str: + ... + + @abstractmethod + def set_path(self, path) -> None: + ... - def __init__(self, path: str): - self.path = path + path = property(get_path, set_path) def __repr__(self): return f"{type(self).__name__}({self.path})" @@ -26,9 +32,10 @@ def __repr__(self): @classmethod @abstractmethod 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`. - pass + """Construct an instance from a path. + + NB: If this concrete method fails to load the data at the given path, then + it should throw a `FileLoadError`.""" class AlignmentFileLoader(FileLoaderBase): diff --git a/ultratrace2/model/files/loaders/dicom.py b/ultratrace2/model/files/loaders/dicom.py index 86670d9..6ae1dc6 100644 --- a/ultratrace2/model/files/loaders/dicom.py +++ b/ultratrace2/model/files/loaders/dicom.py @@ -8,12 +8,18 @@ class DICOMLoader(ImageSetFileLoader): + def get_path(self) -> str: + return self._path + + def set_path(self, path) -> None: + self._path = path + def __init__(self, path: str, pixels: np.ndarray): """Construct the DICOMLoader from a pixel array. The `shape` of the pixel array should be `(n_frames, n_rows, n_columns)` for greyscale images and `(n_frames, n_rows, n_columns, rgb_data)` for full-color images.""" - super().__init__(path) + self.set_path(path) self.pixels = pixels # FIXME: these should be in the `.ultratrace/` dir self.png_dir = f"{self.path}-frames" diff --git a/ultratrace2/model/files/loaders/flac.py b/ultratrace2/model/files/loaders/flac.py index fdd9b75..bc05a13 100644 --- a/ultratrace2/model/files/loaders/flac.py +++ b/ultratrace2/model/files/loaders/flac.py @@ -2,6 +2,15 @@ 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): + self.set_path(path) + @classmethod def from_file(cls, path: str) -> "FLACLoader": raise NotImplementedError() diff --git a/ultratrace2/model/files/loaders/measurement.py b/ultratrace2/model/files/loaders/measurement.py index a586179..f85ba48 100644 --- a/ultratrace2/model/files/loaders/measurement.py +++ b/ultratrace2/model/files/loaders/measurement.py @@ -4,6 +4,15 @@ class MeasurementLoader(AlignmentFileLoader): # FIXME: what is this? do we need to support it? + def get_path(self) -> str: + return self._path + + def set_path(self, path) -> None: + self._path = path + + def __init__(self, path: str): + self.set_path(path) + @classmethod def from_file(cls, path: str) -> "MeasurementLoader": raise NotImplementedError() diff --git a/ultratrace2/model/files/loaders/mp3.py b/ultratrace2/model/files/loaders/mp3.py index b183616..c19df99 100644 --- a/ultratrace2/model/files/loaders/mp3.py +++ b/ultratrace2/model/files/loaders/mp3.py @@ -2,6 +2,15 @@ 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): + self.set_path(path) + @classmethod def from_file(cls, path: str) -> "MP3Loader": raise NotImplementedError() diff --git a/ultratrace2/model/files/loaders/ogg.py b/ultratrace2/model/files/loaders/ogg.py index d70794c..fd52679 100644 --- a/ultratrace2/model/files/loaders/ogg.py +++ b/ultratrace2/model/files/loaders/ogg.py @@ -2,6 +2,15 @@ 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): + self.set_path(path) + @classmethod def from_file(cls, path: str) -> "OggLoader": raise NotImplementedError() diff --git a/ultratrace2/model/files/loaders/textgrid.py b/ultratrace2/model/files/loaders/textgrid.py index b93a6f3..9becb63 100644 --- a/ultratrace2/model/files/loaders/textgrid.py +++ b/ultratrace2/model/files/loaders/textgrid.py @@ -2,6 +2,15 @@ class TextGridLoader(AlignmentFileLoader): + def get_path(self) -> str: + return self._path + + def set_path(self, path) -> None: + self._path = path + + def __init__(self, path: str): + self.set_path(path) + @classmethod def from_file(cls, path: str) -> "TextGridLoader": raise NotImplementedError() diff --git a/ultratrace2/model/files/loaders/wav.py b/ultratrace2/model/files/loaders/wav.py index 4cb593e..2c218ba 100644 --- a/ultratrace2/model/files/loaders/wav.py +++ b/ultratrace2/model/files/loaders/wav.py @@ -2,6 +2,15 @@ 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): + self.set_path(path) + @classmethod def from_file(cls, path: str) -> "WAVLoader": raise NotImplementedError()