fix(api): add missing denormalized rules in refactor export_study_fla… #1676
Conversation
Tests
Code Refactoring
Bug Fixes
ContributorsCommit-Lint commandsYou can trigger Commit-Lint actions by commenting on this PR:
|
laurent-laporte-pro
force-pushed
the
fix/denormalize_export_study_flat
branch
from
8d4dd66 to
02c7f67
Compare
Co-authored-by: LAIDI Takfarinas (Externe) <[email protected]> Co-authored-by: TLAIDI <[email protected]>
| output_list_filter=output_list_filter, | ||
| output_src_path=output_src_path, | ||
| ) | ||
| study = self.study_factory.create_from_fs(dst_path, "", use_cache=False) |
There was a problem hiding this comment.
La "dénormalisation" doit rester optionnelle, il te faut donc conserver le paramètre denormalize.
| @@ -325,28 +324,21 @@ def import_study(self, metadata: RawStudy, stream: IO[bytes]) -> Study: | |||
|
|
|||
| def export_study_flat( | |||
There was a problem hiding this comment.
Attention : la signature de cette fonction publique a changé. Cela entraîne une rupture de compatibilité.
Il faut donc adapter le code appelant (je crois que c'est fait), mais aussi les tests unitaires.
| dst_path: Path, | ||
| outputs: bool = True, | ||
| output_src_path: Optional[Path] = None, |
There was a problem hiding this comment.
Pour moi, ce paramètre est inutile car le répertoire de sortie est et restera toujours "output'.
| shutil.rmtree(output_dest_path) | ||
| if output_list_filter is not None: | ||
| os.mkdir(output_dest_path) | ||
| for output in output_list_filter: |
There was a problem hiding this comment.
Le défaut de cet algorithme réside dans la possibilité de dupliquer des données. En effet, lorsqu'une sortie au format ZIP est décompressée, il n'y a aucune garantie que le fichier ZIP d'origine soit supprimé à la fin de la décompression. Autrement dit, si l'on décompresse le fichier "20230802-1628eco.zip" (par exemple, via une tâche planifiée), il est possible de se retrouver avec à la fois le répertoire "20230802-1628eco" et le fichier "20230802-1628eco.zip" d'origine.
| dst=output_dest_path / output, | ||
| ) | ||
| else: | ||
| shutil.copytree( |
There was a problem hiding this comment.
Ma remarque concernant la duplication des sorties et aussi valable ici.
There was a problem hiding this comment.
Je te propose l'implémentation suivante :
import functools
import shutil
import typing as t
import zipfile
from pathlib import Path
# noinspection PyUnusedLocal
def _ignore_patterns(path_study: Path, directory: str, contents: t.Sequence[str]) -> t.Sequence[str]:
return ["output"] if Path(directory) == path_study else []
def export_study_flat(
path_study: Path,
dest: Path,
outputs: bool = True,
output_list_filter: t.Sequence[str] = (),
) -> None:
"""
Export study to destination
Args:
path_study: Study source path
dest: Destination path.
outputs: List of outputs to keep.
output_list_filter: List of outputs to keep (None indicate all outputs).
"""
output_src_path = path_study / "output"
output_dest_path = dest / "output"
ignore_patterns = functools.partial(_ignore_patterns, path_study)
# noinspection PyTypeChecker
shutil.copytree(src=path_study, dst=dest, ignore=ignore_patterns)
if outputs and output_src_path.exists():
if not output_list_filter:
# Retrieve all directories or ZIP files without duplicates
output_list_filter = list(
{
# fmt: off
f.with_suffix("").name
for f in output_src_path.iterdir()
if f.is_dir() or f.suffix.lower() == ".zip"
# fmt: on
}
)
# Copy each folder or uncompress each ZIP file to the destination dir.
shutil.rmtree(output_dest_path, ignore_errors=True)
output_dest_path.mkdir()
for output in output_list_filter:
zip_path = output_src_path / f"{output}.zip"
if zip_path.exists():
with zipfile.ZipFile(zip_path) as zf:
zf.extractall(output_dest_path / output)
else:
shutil.copytree(
src=output_src_path / output,
dst=output_dest_path / output,
)| dst=output_dest_path, | ||
| ) | ||
|
|
||
| stop_time = time.time() |
There was a problem hiding this comment.
Bof ! on peut supprimer.
| dst_path=dst_path, | ||
| outputs=False, | ||
| denormalize=False, | ||
| output_src_path=output_src_path, |
There was a problem hiding this comment.
Le paramètre output_src_path n'est pas utile (car calculable).
| dst_path: Path, | ||
| outputs: bool = True, | ||
| output_src_path: Optional[Path] = None, |
There was a problem hiding this comment.
Il faut conserver le paramètre denormalize !
…re in 'export_study_flat'
laurent-laporte-pro
force-pushed
the
fix/denormalize_export_study_flat
branch
from
02c7f67 to
abd7490
Compare
|
wont fix |
…t (#1646)