diff --git a/src/osekit/core/base_dataset.py b/src/osekit/core/base_dataset.py index 037d8a6c..ab533cdf 100644 --- a/src/osekit/core/base_dataset.py +++ b/src/osekit/core/base_dataset.py @@ -50,7 +50,6 @@ def __init__( """Instantiate a Dataset object from the Data objects.""" self.data = data self._name = name - self._has_default_name = name is None self._suffix = suffix self._folder = folder @@ -101,11 +100,6 @@ def suffix(self) -> str: def suffix(self, suffix: str | None) -> None: self._suffix = suffix - @property - def has_default_name(self) -> bool: - """Return ``True`` if the dataset has a default name, ``False`` if it has a given name.""" - return self._has_default_name - @property def begin(self) -> Timestamp: """Begin of the first data object.""" diff --git a/src/osekit/core/json_serializer.py b/src/osekit/core/json_serializer.py index e0d2fdb9..1adb56b5 100644 --- a/src/osekit/core/json_serializer.py +++ b/src/osekit/core/json_serializer.py @@ -124,7 +124,8 @@ def serialize_json(path: Path, serialized_dict: dict) -> None: Dictionary to be serialized. """ - path.parent.mkdir(parents=True, exist_ok=True) + if not (parent_folder := path.parent).exists(): + parent_folder.mkdir(parents=True) set_path_reference( serialized_dict=serialized_dict, root_path=path.parent, diff --git a/src/osekit/public/project.py b/src/osekit/public/project.py index 123412bd..6a899a85 100644 --- a/src/osekit/public/project.py +++ b/src/osekit/public/project.py @@ -28,6 +28,7 @@ from osekit.utils.core import ( file_indexes_per_batch, get_umask, + locked, ) from osekit.utils.path import move_tree, ensure_within_base @@ -465,12 +466,41 @@ def run( self.write_json() + @staticmethod + def _reserve_folder(folder: Path) -> None: + """Create a target folder in which the transform outputs will be exported. + + A ``FileExistsError`` is raised if the target folder already exists. + This could happen if a transform with the same name is beeing run + from another process. + + Parameters + ---------- + folder: Path + Folder in which the transform output files will be exported. + + """ + try: + folder.mkdir(parents=True, exist_ok=False) + except FileExistsError as e: + msg = ( + f"Target folder {folder} already exists.\n" + f"It might mean that another process already ran a transform that" + f"exports in this folder.\n" + f"Change the current transform name or use the" + f"Project.delete_transform_with_outputs() or" + f"Project.rename_transform_with_outputs() method." + ) + raise FileExistsError(msg) from e + def _add_audio_dataset( self, ads: AudioDataset, transform_name: str, ) -> None: ads.folder = self._get_audio_dataset_subpath(ads=ads) + self._reserve_folder(folder=ads.folder) + self.outputs[ads.name] = { "class": type(ads).__name__, "transform": transform_name, @@ -482,16 +512,7 @@ def _get_audio_dataset_subpath( self, ads: AudioDataset, ) -> Path: - return ( - self.folder - / self.SUBFOLDERS["data"] - / "audio" - / ( - f"{round(ads.data_duration.total_seconds())}_{round(ads.sample_rate)}" - if ads.has_default_name - else ads.name - ) - ) + return self.folder / self.SUBFOLDERS["data"] / "audio" / ads.name def export( self, @@ -621,6 +642,7 @@ def _add_spectro_dataset( transform_name: str, ) -> None: sds.folder = self._get_spectro_dataset_subpath(sds=sds) + self._reserve_folder(folder=sds.folder) self.outputs[sds.name] = { "class": type(sds).__name__, "dataset": sds, @@ -632,15 +654,7 @@ def _get_spectro_dataset_subpath( self, sds: SpectroDataset | LTASDataset, ) -> Path: - ads_folder = Path( - f"{round(sds.data_duration.total_seconds())}_{round(sds.fft.fs)}", - ) - fft_folder = f"{sds.fft.mfft}_{sds.fft.win.shape[0]}_{sds.fft.hop}_linear" - return ( - self.folder - / self.SUBFOLDERS["processed"] - / (ads_folder / fft_folder if sds.has_default_name else sds.name) - ) + return self.folder / self.SUBFOLDERS["processed"] / sds.name def _sort_dataset(self, dataset: type[DatasetChild]) -> None: if type(dataset) is AudioDataset: @@ -678,7 +692,7 @@ def _delete_output(self, output_dataset_name: str) -> None: afm.close() shutil.rmtree(str(output_to_remove.folder)) - self.write_json() + self.write_json(output_to_skip=output_to_remove.name) def get_output_by_transform_name( self, @@ -820,7 +834,7 @@ def to_dict(self) -> dict: if isinstance(dataset["dataset"], Path) else str(dataset["dataset"].folder / f"{name}.json"), } - for name, dataset in self.outputs.items() + for name, dataset in sorted(self.outputs.items(), key=lambda kv: kv[0]) }, "instrument": ( None if self.instrument is None else self.instrument.to_dict() @@ -863,10 +877,28 @@ def from_dict(cls, dictionary: dict) -> Project: outputs=outputs, ) - def write_json(self, folder: Path | None = None) -> None: + def write_json( + self, + folder: Path | None = None, + output_to_skip: str | None = None, + ) -> None: """Write a serialized Project to a JSON file.""" folder = folder if folder is not None else self.folder - serialize_json(folder / "project.json", self.to_dict()) + json_file = folder / "project.json" + + @locked(lock_file=folder / "project.lock") + def _write() -> None: + dictionary = self.to_dict() + if json_file.exists(): + # Update outputs in case there are unexisting keys in the dictionary. + existing_outputs = deserialize_json(path=json_file).get("outputs", {}) + if output_to_skip and output_to_skip in existing_outputs: + existing_outputs.pop(output_to_skip) + dictionary["outputs"] |= existing_outputs + + serialize_json(folder / "project.json", dictionary) + + _write() @classmethod def from_json(cls, file: Path) -> Project: diff --git a/src/osekit/public/transform.py b/src/osekit/public/transform.py index 895cd01c..8fcc4b57 100644 --- a/src/osekit/public/transform.py +++ b/src/osekit/public/transform.py @@ -66,6 +66,7 @@ class Transform: def __init__( self, output_type: OutputType, + name: str, begin: Timestamp | None = None, end: Timestamp | None = None, data_duration: Timedelta | None = None, @@ -74,7 +75,6 @@ def __init__( sample_rate: float | None = None, normalization: Normalization = Normalization.RAW, butter: Butterworth | None = None, - name: str | None = None, subtype: str | None = None, fft: ShortTimeFFT | None = None, v_lim: tuple[float, float] | None = None, @@ -89,6 +89,8 @@ def __init__( output_type: OutputType The type of transform to run. See ``OutputType`` docstring for more info. + name: str | None + Name of the transform dataset. begin: Timestamp | None The begin of the transform dataset. Defaulted to the begin of the original dataset. @@ -121,11 +123,6 @@ def __init__( The type of normalization to apply to the audio data. butter: Butterworth | None Butterworth filter to apply to the audio data. - name: str | None - Name of the transform dataset. - Defaulted as the begin timestamp of the transform dataset. - If both audio and spectro outputs are selected, the audio - transform dataset name will be suffixed with ``"_audio"``. subtype: str | None Subtype of the written audio files as provided by the soundfile module. Defaulted as the default ``16-bit PCM`` for ``wav`` audio files. diff --git a/tests/test_core_api_base.py b/tests/test_core_api_base.py index 1946996e..1fc4dcf7 100644 --- a/tests/test_core_api_base.py +++ b/tests/test_core_api_base.py @@ -796,6 +796,61 @@ def test_base_dataset_from_folder( assert np.array_equal(sorted(f.path for f in data.files), sorted(expected[1])) +@pytest.mark.parametrize( + ("data_durations", "expected_duration"), + [ + pytest.param( + [Timedelta(seconds=10)], + Timedelta(seconds=10), + id="only_one_data", + ), + pytest.param( + [ + Timedelta(seconds=10), + Timedelta(seconds=10), + Timedelta(seconds=10), + ], + Timedelta(seconds=10), + id="all_data_have_the_same_duration", + ), + pytest.param( + [ + Timedelta(seconds=15), + Timedelta(seconds=15), + Timedelta(seconds=10), + ], + Timedelta(seconds=15), + id="dataset_duration_is_most_frequent_one", + ), + pytest.param( + [ + Timedelta(seconds=10), + Timedelta(seconds=20), + Timedelta(seconds=15), + ], + Timedelta(seconds=20), + id="only_one_of_each_duration_takes_the_longest", + ), + ], +) +def test_base_dataset_data_duration( + data_durations: list[Timedelta], expected_duration: Timedelta +) -> None: + files = [] + for data_duration in data_durations: + df = DummyFile( + path=Path(), + begin=Timestamp("1994-09-27 00:00:00") + + Timedelta(seconds=sum(f.duration.total_seconds() for f in files)), + ) + df.end = df.begin + data_duration + files.append(df) + + assert ( + DummyDataset.from_files(files, mode="files").data_duration == expected_duration + ) + + @pytest.mark.parametrize( "destination_folder", [ diff --git a/tests/test_public_api.py b/tests/test_public_api.py index 27d27ae4..0b598ded 100644 --- a/tests/test_public_api.py +++ b/tests/test_public_api.py @@ -1,6 +1,7 @@ from __future__ import annotations import itertools +import json from contextlib import AbstractContextManager, nullcontext from copy import deepcopy from pathlib import Path @@ -332,18 +333,6 @@ def test_project_build( @pytest.mark.parametrize( "transform", [ - pytest.param( - Transform( - output_type=OutputType.AUDIO, - name=None, - begin=None, - end=None, - data_duration=None, - sample_rate=None, - subtype="DOUBLE", - ), - id="same_format_as_original", - ), pytest.param( Transform( output_type=OutputType.AUDIO, @@ -427,14 +416,9 @@ def test_reshape( if transform.sample_rate is not None: expected_ads.sample_rate = transform.sample_rate - expected_ads_name = ( - transform.name - or f"{expected_ads.begin.strftime(TIMESTAMP_FORMAT_EXPORTED_FILES_UNLOCALIZED)}" - ) - # The new dataset should be added to the outputs property - assert expected_ads_name in project.outputs - ads = project.get_output(expected_ads_name) + assert transform.name in project.outputs + ads = project.get_output(transform.name) assert ads is not None assert type(ads) is AudioDataset @@ -449,17 +433,13 @@ def test_reshape( ) # ads folder should match the ads name - ads_folder_name = ( - transform.name - or f"{round(ads.data_duration.total_seconds())}_{ads.sample_rate}" - ) - assert ads.folder.name == ads_folder_name + assert ads.folder.name == transform.name # ads should be linked to the new files instead of the originals assert all(file not in project.origin_files for file in ads.files) # ads should be deserializable from the exported JSON file - json_file = ads.folder / f"{expected_ads_name}.json" + json_file = ads.folder / f"{transform.name}.json" assert json_file.exists() deserialized_ads = AudioDataset.from_json(json_file) assert deserialized_ads == ads @@ -612,6 +592,7 @@ def test_spectral_transform_error_if_no_provided_fft(output_type: OutputType) -> ): Transform( output_type=OutputType.SPECTROGRAM, + name="magnetic_fields", ) @@ -619,13 +600,14 @@ def test_spectral_transform_error_if_no_provided_fft(output_type: OutputType) -> ("transform", "expected"), [ pytest.param( - Transform(output_type=OutputType.AUDIO), + Transform(output_type=OutputType.AUDIO, name="cool"), False, id="audio_only", ), pytest.param( Transform( output_type=OutputType.SPECTROGRAM, + name="cool", fft=ShortTimeFFT(hamming(1024), 1024, 48_000), ), True, @@ -634,6 +616,7 @@ def test_spectral_transform_error_if_no_provided_fft(output_type: OutputType) -> pytest.param( Transform( output_type=OutputType.SPECTRUM, + name="cool", fft=ShortTimeFFT(hamming(1024), 1024, 48_000), ), True, @@ -642,6 +625,7 @@ def test_spectral_transform_error_if_no_provided_fft(output_type: OutputType) -> pytest.param( Transform( output_type=OutputType.SPECTRUM | OutputType.SPECTROGRAM, + name="cool", fft=ShortTimeFFT(hamming(1024), 1024, 48_000), ), True, @@ -652,6 +636,7 @@ def test_spectral_transform_error_if_no_provided_fft(output_type: OutputType) -> output_type=OutputType.SPECTRUM | OutputType.SPECTROGRAM | OutputType.AUDIO, + name="cool", fft=ShortTimeFFT(hamming(1024), 1024, 48_000), ), True, @@ -670,6 +655,7 @@ def test_transform_constructor_rejects_mismatched_fs() -> None: ): Transform( output_type=OutputType.SPECTROGRAM, + name="cool", sample_rate=32_000, fft=ShortTimeFFT(hamming(1024), 1024, 48_000), ) @@ -678,6 +664,7 @@ def test_transform_constructor_rejects_mismatched_fs() -> None: def test_transform_rejects_setting_fft_with_wrong_fs() -> None: transform = Transform( output_type=OutputType.SPECTROGRAM, + name="cool", sample_rate=48_000, fft=ShortTimeFFT(hamming(1024), 1024, 48_000), ) @@ -692,6 +679,7 @@ def test_transform_rejects_setting_fft_with_wrong_fs() -> None: def test_transform_sample_rate_propagates_to_fft() -> None: transform = Transform( output_type=OutputType.SPECTROGRAM, + name="cool", sample_rate=48_000, fft=ShortTimeFFT(hamming(1024), 1024, 48_000), ) @@ -744,7 +732,7 @@ def test_transform_validate_sample_rate( expected: AbstractContextManager, ) -> None: with expected: - Transform(OutputType.AUDIO)._validate_sample_rate( + Transform(OutputType.AUDIO, name="cool")._validate_sample_rate( sample_rate=sample_rate, fft=fft, ) @@ -772,7 +760,7 @@ def test_transform_validate_sample_rate( None, Transform( output_type=OutputType.AUDIO, - name=None, + name="cool", begin=None, end=None, data_duration=None, @@ -785,7 +773,7 @@ def test_transform_validate_sample_rate( end=Timestamp("2024-01-01 12:00:05"), ), ], - id="no_transform_name", + id="no_project_instrument", ), pytest.param( Instrument(end_to_end_db=150), @@ -1095,6 +1083,7 @@ def test_prepare_spectro( project.prepare_spectro( transform=Transform( output_type=OutputType.SPECTROGRAM, + name="cool", ), ) @@ -1225,18 +1214,18 @@ def test_delete_output_dataset( datasets = [ds1, ds2, ds3, ds4] - for i, ds in enumerate(datasets): - assert ds.name in project.outputs.keys() + for ds in datasets: + assert ds.name in project.outputs assert ds.folder.exists() project._delete_output(str(ds.name)) - assert ds.name not in project.outputs.keys() + assert ds.name not in project.outputs assert not ds.folder.exists() # The JSON should be updated new_project = Project.from_json(project.folder / "project.json") - assert ds.name not in new_project.outputs.keys() + assert ds.name not in new_project.outputs @pytest.mark.parametrize( @@ -1759,3 +1748,76 @@ def mock_from_json( # Getting the dataset again should use the cached dataset _ = project.get_output("ltas") assert json_calls[0] == 3 + + +def test_project_json_update( + sample_project: tuple[Project, pytest.fixtures.Subrequest], + dummy_export_transform: None, + patch_afm_info: None, +) -> None: + project, _ = sample_project + + json_file = project.folder / "project.json" + + # We simulate another job writing in the JSON file without sample_project knowing + # about it + with json_file.open("r") as f: + data = json.load(f) + + data["outputs"]["ghost_transform"] = { + "class": "AudioDataset", + "transform": "ghost_transform", + "json": "data/audio/ghost_transform/ghost_transform.json", + } + + with json_file.open("w") as f: + json.dump(data, f) + + # Run a transform from the instance that ignores ghost_transform + project.run( + transform=Transform( + output_type=OutputType.AUDIO, + name="new_transform", + ), + ) + + with json_file.open("r") as f: + data_after = json.load(f) + + assert "ghost_transform" in data_after["outputs"] + assert "new_transform" in data_after["outputs"] + assert "original" in data_after["outputs"] + + +def test_run_transform_with_same_name_in_different_process( + sample_project: tuple[Project, pytest.fixtures.Subrequest], + dummy_export_transform: None, + patch_afm_info: None, +) -> None: + project, _ = sample_project + + # We simulate another job running a transform in a different process, + # without this process knowing about it: + # exported file, updated JSONs, but unregistered transforms and outputs fields. + transform = Transform( + output_type=OutputType.AUDIO | OutputType.SPECTROGRAM, + name="part_company", + fft=ShortTimeFFT( + win=hamming(1024), + hop=512, + fs=project.origin_dataset.sample_rate, + ), + ) + project.run(transform=transform) + for output_name in ( + output.name for output in project.get_output_by_transform_name("part_company") + ): + del project.outputs[output_name] + + # Running a transform that exports in the already existing folders should raise: + with pytest.raises(FileExistsError, match="already exists"): + project.run(transform=transform) + + transform.output_type = OutputType.SPECTROGRAM + with pytest.raises(FileExistsError, match="already exists"): + project.run(transform=transform) diff --git a/tests/test_serialization.py b/tests/test_serialization.py index 6d2ba6df..82460c97 100644 --- a/tests/test_serialization.py +++ b/tests/test_serialization.py @@ -365,7 +365,6 @@ def test_audio_dataset_serialization( assert str(ads) == str(ads2) assert ads.name == ads2.name - assert ads.has_default_name == ads2.has_default_name assert ads.sample_rate == ads2.sample_rate assert ads.begin == ads2.begin assert ads.normalization == ads2.normalization @@ -935,7 +934,6 @@ def test_spectro_dataset_serialization( assert sds.name == sds2.name assert sds.colormap == sds2.colormap assert sds.scale == sds2.scale - assert sds.has_default_name == sds2.has_default_name assert sds.begin == sds2.begin assert all( np.array_equal(sd.get_value(), sd2.get_value()) @@ -993,7 +991,6 @@ def test_spectro_dataset_serialization( assert sds.name == sds4.name assert sds.colormap == sds4.colormap assert sds.scale == sds4.scale - assert sds.has_default_name == sds4.has_default_name assert sds.begin == sds4.begin