From 46bcdb1b7150ae610a5b35532c5a46b672b93697 Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Wed, 11 May 2022 20:19:27 -0400 Subject: [PATCH 01/30] Built-in BIDS support for `dandi upload` --- dandi/cli/cmd_validate.py | 30 +------------------ dandi/upload.py | 63 +++++++++++++++++++++++++++++++++++++++ dandi/validate.py | 36 +++++++++++++++++++++- 3 files changed, 99 insertions(+), 30 deletions(-) diff --git a/dandi/cli/cmd_validate.py b/dandi/cli/cmd_validate.py index 5fcbede87..88897b060 100644 --- a/dandi/cli/cmd_validate.py +++ b/dandi/cli/cmd_validate.py @@ -4,7 +4,6 @@ import click from .base import devel_debug_option, devel_option, lgr, map_to_click_exceptions -from ..utils import pluralize @click.command() @@ -31,39 +30,12 @@ def validate_bids( if report_flag and not report: report = report_flag - validation_result = validate_bids_( + _ = validate_bids_( *paths, report=report, schema_version=schema, devel_debug=devel_debug, ) - missing_files = [ - pattern["regex"] - for pattern in validation_result["schema_tracking"] - if pattern["mandatory"] - ] - error_list = [] - if missing_files: - error_substring = ( - f"{pluralize(len(missing_files), 'filename pattern')} required " - "by BIDS could not be found" - ) - error_list.append(error_substring) - if validation_result["path_tracking"]: - error_substring = ( - f"{pluralize(len(validation_result['path_tracking']), 'filename')} " - "did not match any pattern known to BIDS" - ) - error_list.append(error_substring) - if error_list: - error_string = " and ".join(error_list) - error_string = f"Summary: {error_string}." - click.secho( - error_string, - bold=True, - fg="red", - ) - raise SystemExit(1) @click.command() diff --git a/dandi/upload.py b/dandi/upload.py index 56ccef1ae..9dd9abc0a 100644 --- a/dandi/upload.py +++ b/dandi/upload.py @@ -35,6 +35,11 @@ class Uploaded(TypedDict): errors: List[str] +class BIDSValidationError(Exception): + pass + # raise Exception("BIDS Dataset is not valid.") + + def upload( paths: Optional[List[Union[str, Path]]] = None, existing: str = "refresh", @@ -55,6 +60,21 @@ def upload( dandiset_ = Dandiset.find(os.path.commonpath(paths)) else: dandiset_ = Dandiset.find(None) + + # pre-validate BIDS datasets before going for individual + # files etc + bids_datasets = _bids_discover_and_validate(dandiset_.path, paths, validation) + + if bids_datasets: + bids_datasets = [str(i) for i in bids_datasets] + if not allow_any_path: + lgr.info( + "Setting allow_any_path to True since we detected %s under: %s", + pluralize(len(bids_datasets), "BIDS dataset"), + ", ".join(bids_datasets), + ) + allow_any_path = True + if not dandiset_: raise RuntimeError( f"Found no {dandiset_metadata_file} anywhere in common ancestor of" @@ -383,3 +403,46 @@ def check_replace_asset( def skip_file(msg: Any) -> Dict[str, str]: return {"status": "skipped", "message": str(msg)} + + +def _bids_discover_and_validate(dandiset_path, paths, validation): + """Temporary implementation for discovery and validation of BIDS datasets + + References: + - unification of validation records: https://github.com/dandi/dandi-cli/issues/943 + - validation "design doc": https://github.com/dandi/dandi-cli/pull/663 + """ + from .utils import find_files + from .validate import validate_bids + + if paths: + bids_lookup_paths = set(p for p in paths) + else: + bids_lookup_paths = None + bids_descriptions = map(Path, find_files("dataset_description.json", dandiset_path)) + bids_datasets = [p.parent for p in bids_descriptions] + if bids_datasets: + lgr.debug( + "Detected %d BIDS datasets at following paths: %s", + len(bids_datasets), + ", ".join([str(i) for i in bids_datasets]), + ) + + if validation != "skip": + if bids_lookup_paths: + bids_datasets_to_validate = set() + for p in bids_lookup_paths: + for bd in bids_datasets: + if p.is_relative_to(bd): + bids_datasets_to_validate.add(bd) + break + else: + bids_datasets_to_validate = bids_datasets + bids_datasets_to_validate = sorted(bids_datasets_to_validate) + for bd in sorted(bids_datasets_to_validate): + _ = validate_bids( + bd, allow_errors=True if validation == "ignore" else False + ) + return bids_datasets_to_validate + else: + return bids_datasets diff --git a/dandi/validate.py b/dandi/validate.py index 6270af743..a41e71217 100644 --- a/dandi/validate.py +++ b/dandi/validate.py @@ -1,6 +1,9 @@ from typing import Any, Iterator, List, Optional, Tuple +import click + from .files import find_dandi_files +from .utils import pluralize # TODO: provide our own "errors" records, which would also include warnings etc @@ -10,6 +13,7 @@ def validate_bids( schema_version: Optional[str] = None, devel_debug: bool = False, report: Optional[str] = None, + allow_errors: Optional[bool] = False, ) -> Any: """Validate BIDS paths. @@ -25,6 +29,8 @@ def validate_bids( If `True` a log will be written using the standard output path of `.write_report()`. If string, the string will be used as the output path. If the variable evaluates as False, no log will be written. + allow_errors : bool, optional + Whether to raise errors on invalid dataset. Notes ----- @@ -33,9 +39,37 @@ def validate_bids( """ from .bids_validator_xs import validate_bids as validate_bids_ - return validate_bids_( + validation_result = validate_bids_( paths, schema_version=schema_version, debug=devel_debug, report_path=report ) + missing_files = [ + pattern["regex"] + for pattern in validation_result["schema_tracking"] + if pattern["mandatory"] + ] + error_list = [] + if missing_files: + error_substring = ( + f"{pluralize(len(missing_files), 'filename pattern')} required " + "by BIDS could not be found" + ) + error_list.append(error_substring) + if validation_result["path_tracking"]: + error_substring = ( + f"{pluralize(len(validation_result['path_tracking']), 'filename')} " + "did not match any pattern known to BIDS" + ) + error_list.append(error_substring) + if error_list: + error_string = " and ".join(error_list) + error_string = f"Summary: {error_string}." + click.secho( + error_string, + bold=True, + fg="red", + ) + if not allow_errors: + raise SystemExit(1) def validate( From d508c7b334886d3647270e8fdff252fa208a66af Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Thu, 12 May 2022 21:52:57 -0400 Subject: [PATCH 02/30] fixed logic for None input --- dandi/upload.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/dandi/upload.py b/dandi/upload.py index 9dd9abc0a..af5da64eb 100644 --- a/dandi/upload.py +++ b/dandi/upload.py @@ -60,6 +60,11 @@ def upload( dandiset_ = Dandiset.find(os.path.commonpath(paths)) else: dandiset_ = Dandiset.find(None) + if not dandiset_: + raise RuntimeError( + f"Found no {dandiset_metadata_file} anywhere in common ancestor of" + " paths. Use 'dandi download' or 'organize' first." + ) # pre-validate BIDS datasets before going for individual # files etc @@ -75,12 +80,6 @@ def upload( ) allow_any_path = True - if not dandiset_: - raise RuntimeError( - f"Found no {dandiset_metadata_file} anywhere in common ancestor of" - " paths. Use 'dandi download' or 'organize' first." - ) - instance = get_instance(dandi_instance) assert instance.api is not None api_url = instance.api From 5531d3c129285d29bf6d18882cde78942c2b4a2c Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Fri, 13 May 2022 05:49:55 -0400 Subject: [PATCH 03/30] not using return value --- dandi/cli/cmd_validate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dandi/cli/cmd_validate.py b/dandi/cli/cmd_validate.py index 88897b060..814fbf76c 100644 --- a/dandi/cli/cmd_validate.py +++ b/dandi/cli/cmd_validate.py @@ -30,7 +30,7 @@ def validate_bids( if report_flag and not report: report = report_flag - _ = validate_bids_( + validate_bids_( *paths, report=report, schema_version=schema, From db3379c62c9065918541cdf1c9a377890b872b9c Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Mon, 6 Jun 2022 10:41:27 -0400 Subject: [PATCH 04/30] Code improvements as suggested by jwodder --- dandi/upload.py | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/dandi/upload.py b/dandi/upload.py index af5da64eb..1a529535f 100644 --- a/dandi/upload.py +++ b/dandi/upload.py @@ -35,11 +35,6 @@ class Uploaded(TypedDict): errors: List[str] -class BIDSValidationError(Exception): - pass - # raise Exception("BIDS Dataset is not valid.") - - def upload( paths: Optional[List[Union[str, Path]]] = None, existing: str = "refresh", @@ -74,7 +69,8 @@ def upload( bids_datasets = [str(i) for i in bids_datasets] if not allow_any_path: lgr.info( - "Setting allow_any_path to True since we detected %s under: %s", + "Setting allow_any_path to True since we detected %s under the " + "following paths: %s", pluralize(len(bids_datasets), "BIDS dataset"), ", ".join(bids_datasets), ) @@ -404,7 +400,11 @@ def skip_file(msg: Any) -> Dict[str, str]: return {"status": "skipped", "message": str(msg)} -def _bids_discover_and_validate(dandiset_path, paths, validation): +def _bids_discover_and_validate( + dandiset_path: str, + paths: Optional[List[Union[str, Path]]] = None, + validation: str = "require", +): """Temporary implementation for discovery and validation of BIDS datasets References: @@ -415,33 +415,37 @@ def _bids_discover_and_validate(dandiset_path, paths, validation): from .validate import validate_bids if paths: - bids_lookup_paths = set(p for p in paths) + bids_lookup_paths = set(paths) else: bids_lookup_paths = None - bids_descriptions = map(Path, find_files("dataset_description.json", dandiset_path)) + bids_descriptions = map( + Path, find_files(r"dataset_description\.json$", dandiset_path) + ) bids_datasets = [p.parent for p in bids_descriptions] if bids_datasets: lgr.debug( - "Detected %d BIDS datasets at following paths: %s", - len(bids_datasets), - ", ".join([str(i) for i in bids_datasets]), + "Detected %s under following paths: %s", + pluralize(len(bids_datasets), "BIDS dataset"), + ", ".join(str(i) for i in bids_datasets), ) if validation != "skip": if bids_lookup_paths: - bids_datasets_to_validate = set() + bids_datasets_to_validate = list() for p in bids_lookup_paths: for bd in bids_datasets: - if p.is_relative_to(bd): - bids_datasets_to_validate.add(bd) + try: + p.relative_to(bd) + except ValueError: + pass + else: + bids_datasets_to_validate.append(bd) break else: bids_datasets_to_validate = bids_datasets bids_datasets_to_validate = sorted(bids_datasets_to_validate) - for bd in sorted(bids_datasets_to_validate): - _ = validate_bids( - bd, allow_errors=True if validation == "ignore" else False - ) + for bd in bids_datasets_to_validate: + _ = validate_bids(bd, allow_errors=validation == "ignore") return bids_datasets_to_validate else: return bids_datasets From 0890e0a7267d9803a30eb37161d6953aaf0240a3 Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Tue, 7 Jun 2022 06:00:13 -0400 Subject: [PATCH 05/30] Code style and no SystemExit in library function. --- dandi/cli/cmd_validate.py | 4 +++- dandi/upload.py | 13 ++++++++----- dandi/validate.py | 3 ++- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/dandi/cli/cmd_validate.py b/dandi/cli/cmd_validate.py index 814fbf76c..7229b5c96 100644 --- a/dandi/cli/cmd_validate.py +++ b/dandi/cli/cmd_validate.py @@ -30,12 +30,14 @@ def validate_bids( if report_flag and not report: report = report_flag - validate_bids_( + validated = validate_bids_( *paths, report=report, schema_version=schema, devel_debug=devel_debug, ) + if not validated: + raise SystemExit(1) @click.command() diff --git a/dandi/upload.py b/dandi/upload.py index 1a529535f..ad26eb5c6 100644 --- a/dandi/upload.py +++ b/dandi/upload.py @@ -404,7 +404,7 @@ def _bids_discover_and_validate( dandiset_path: str, paths: Optional[List[Union[str, Path]]] = None, validation: str = "require", -): +) -> List[str]: """Temporary implementation for discovery and validation of BIDS datasets References: @@ -419,7 +419,7 @@ def _bids_discover_and_validate( else: bids_lookup_paths = None bids_descriptions = map( - Path, find_files(r"dataset_description\.json$", dandiset_path) + Path, find_files(r"(^|[/\x5C])dataset_description\.json$", dandiset_path) ) bids_datasets = [p.parent for p in bids_descriptions] if bids_datasets: @@ -443,9 +443,12 @@ def _bids_discover_and_validate( break else: bids_datasets_to_validate = bids_datasets - bids_datasets_to_validate = sorted(bids_datasets_to_validate) + bids_datasets_to_validate.sort() + validated_datasets = [] for bd in bids_datasets_to_validate: - _ = validate_bids(bd, allow_errors=validation == "ignore") - return bids_datasets_to_validate + validated = validate_bids(bd, allow_errors=validation == "ignore") + if validated: + validated_datasets.append(bd) + return validated_datasets else: return bids_datasets diff --git a/dandi/validate.py b/dandi/validate.py index a41e71217..b5b91fb4a 100644 --- a/dandi/validate.py +++ b/dandi/validate.py @@ -69,7 +69,8 @@ def validate_bids( fg="red", ) if not allow_errors: - raise SystemExit(1) + return False + return True def validate( From 7ba78d51dc2412359c02fd6928a4f68e22d20c39 Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Tue, 7 Jun 2022 06:18:34 -0400 Subject: [PATCH 06/30] Corrected return type --- dandi/upload.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dandi/upload.py b/dandi/upload.py index ad26eb5c6..3a954cdb0 100644 --- a/dandi/upload.py +++ b/dandi/upload.py @@ -404,7 +404,7 @@ def _bids_discover_and_validate( dandiset_path: str, paths: Optional[List[Union[str, Path]]] = None, validation: str = "require", -) -> List[str]: +) -> List[Path]: """Temporary implementation for discovery and validation of BIDS datasets References: From 9ae2218a89d215f17507115d6b6a23801be096e3 Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Wed, 8 Jun 2022 02:19:47 -0400 Subject: [PATCH 07/30] Fixed typing --- dandi/upload.py | 10 ++++++---- dandi/validate.py | 5 +++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/dandi/upload.py b/dandi/upload.py index 3a954cdb0..5486cb9ce 100644 --- a/dandi/upload.py +++ b/dandi/upload.py @@ -66,13 +66,13 @@ def upload( bids_datasets = _bids_discover_and_validate(dandiset_.path, paths, validation) if bids_datasets: - bids_datasets = [str(i) for i in bids_datasets] + _bids_datasets = [str(i) for i in bids_datasets] if not allow_any_path: lgr.info( "Setting allow_any_path to True since we detected %s under the " "following paths: %s", - pluralize(len(bids_datasets), "BIDS dataset"), - ", ".join(bids_datasets), + pluralize(len(_bids_datasets), "BIDS dataset"), + ", ".join(_bids_datasets), ) allow_any_path = True @@ -403,7 +403,7 @@ def skip_file(msg: Any) -> Dict[str, str]: def _bids_discover_and_validate( dandiset_path: str, paths: Optional[List[Union[str, Path]]] = None, - validation: str = "require", + validation: Optional[str] = "require", ) -> List[Path]: """Temporary implementation for discovery and validation of BIDS datasets @@ -433,6 +433,8 @@ def _bids_discover_and_validate( if bids_lookup_paths: bids_datasets_to_validate = list() for p in bids_lookup_paths: + if isinstance(p, str): + p = Path(p) for bd in bids_datasets: try: p.relative_to(bd) diff --git a/dandi/validate.py b/dandi/validate.py index b5b91fb4a..bf4fa3460 100644 --- a/dandi/validate.py +++ b/dandi/validate.py @@ -1,4 +1,5 @@ -from typing import Any, Iterator, List, Optional, Tuple +from pathlib import Path +from typing import Any, Iterator, List, Optional, Tuple, Union import click @@ -9,7 +10,7 @@ def validate_bids( - *paths: str, + *paths: Union[str, Path], schema_version: Optional[str] = None, devel_debug: bool = False, report: Optional[str] = None, From d69880485eb227776e9bfce7a8124915048d6f61 Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Thu, 9 Jun 2022 00:30:26 -0400 Subject: [PATCH 08/30] Better user prompt --- dandi/upload.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dandi/upload.py b/dandi/upload.py index 5486cb9ce..501fba7ee 100644 --- a/dandi/upload.py +++ b/dandi/upload.py @@ -69,8 +69,8 @@ def upload( _bids_datasets = [str(i) for i in bids_datasets] if not allow_any_path: lgr.info( - "Setting allow_any_path to True since we detected %s under the " - "following paths: %s", + "Enabling --allow-any-path since we detected %s under the following " + "paths: %s", pluralize(len(_bids_datasets), "BIDS dataset"), ", ".join(_bids_datasets), ) From 953f7b450585ad9f5cefa6ea191fec5f04ac56b3 Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Thu, 9 Jun 2022 00:30:36 -0400 Subject: [PATCH 09/30] Validation return typing and docstring --- dandi/validate.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dandi/validate.py b/dandi/validate.py index bf4fa3460..bd72f47d6 100644 --- a/dandi/validate.py +++ b/dandi/validate.py @@ -1,5 +1,5 @@ from pathlib import Path -from typing import Any, Iterator, List, Optional, Tuple, Union +from typing import Iterator, List, Optional, Tuple, Union import click @@ -15,7 +15,7 @@ def validate_bids( devel_debug: bool = False, report: Optional[str] = None, allow_errors: Optional[bool] = False, -) -> Any: +) -> bool: """Validate BIDS paths. Parameters @@ -33,6 +33,11 @@ def validate_bids( allow_errors : bool, optional Whether to raise errors on invalid dataset. + Returns + ------- + bool + Whether or not there were zero validation errors. + Notes ----- Can be used from bash, as: From adf260501fb743466c2276368ddf9fec9288c308 Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Fri, 10 Jun 2022 11:58:40 -0400 Subject: [PATCH 10/30] Allowing upload of BIDS datasets relative to input path --- dandi/bids_validator_xs.py | 25 ++++++++++++++++++------- dandi/upload.py | 10 +++++++--- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/dandi/bids_validator_xs.py b/dandi/bids_validator_xs.py index 13da4ad8a..0ed382ce0 100644 --- a/dandi/bids_validator_xs.py +++ b/dandi/bids_validator_xs.py @@ -611,17 +611,28 @@ def select_schema_dir( ) else: with open(dataset_description) as f: - dataset_info = json.load(f) try: - schema_version = dataset_info["BIDSVersion"] - except KeyError: - lgr.warning( - "BIDSVersion is not specified in " - "`dataset_description.json`. " - "Falling back to %s.", + dataset_info = json.load(f) + except json.decoder.JSONDecodeError: + lgr.error( + "The `%s` file could not be loaded. " + "Please check whether the file is valid JSON. " + "Falling back to the %s BIDS version.", + dataset_description, schema_min_version, ) schema_version = schema_min_version + else: + try: + schema_version = dataset_info["BIDSVersion"] + except KeyError: + lgr.warning( + "BIDSVersion is not specified in " + "`dataset_description.json`. " + "Falling back to %s.", + schema_min_version, + ) + schema_version = schema_min_version if not schema_version: lgr.warning( "No BIDSVersion could be found for the dataset. Falling back to %s.", diff --git a/dandi/upload.py b/dandi/upload.py index 501fba7ee..8667e02cb 100644 --- a/dandi/upload.py +++ b/dandi/upload.py @@ -421,7 +421,7 @@ def _bids_discover_and_validate( bids_descriptions = map( Path, find_files(r"(^|[/\x5C])dataset_description\.json$", dandiset_path) ) - bids_datasets = [p.parent for p in bids_descriptions] + bids_datasets = [bd.parent for bd in bids_descriptions] if bids_datasets: lgr.debug( "Detected %s under following paths: %s", @@ -439,10 +439,14 @@ def _bids_discover_and_validate( try: p.relative_to(bd) except ValueError: - pass + try: + bd.relative_to(p) + except ValueError: + pass + else: + bids_datasets_to_validate.append(bd) else: bids_datasets_to_validate.append(bd) - break else: bids_datasets_to_validate = bids_datasets bids_datasets_to_validate.sort() From e10c8ea477b4eeb12c56e9b7e847887038ad761a Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Mon, 20 Jun 2022 13:47:24 -0400 Subject: [PATCH 11/30] No click usage outside of CLI library --- dandi/bids_utils.py | 42 +++++++++++++++++++++++++++++++++++++++ dandi/cli/cmd_validate.py | 7 +++++-- dandi/upload.py | 10 ++++++++-- dandi/validate.py | 38 ++--------------------------------- 4 files changed, 57 insertions(+), 40 deletions(-) create mode 100644 dandi/bids_utils.py diff --git a/dandi/bids_utils.py b/dandi/bids_utils.py new file mode 100644 index 000000000..466236b02 --- /dev/null +++ b/dandi/bids_utils.py @@ -0,0 +1,42 @@ +from typing import Optional + +import click + +from .utils import pluralize + + +def evaluate_validation( + validation_result: dict, + allow_errors: Optional[bool] = False, + cli_output: Optional[bool] = False, +) -> bool: + missing_files = [ + pattern["regex"] + for pattern in validation_result["schema_tracking"] + if pattern["mandatory"] + ] + error_list = [] + if missing_files: + error_substring = ( + f"{pluralize(len(missing_files), 'filename pattern')} required " + "by BIDS could not be found" + ) + error_list.append(error_substring) + if validation_result["path_tracking"]: + error_substring = ( + f"{pluralize(len(validation_result['path_tracking']), 'filename')} " + "did not match any pattern known to BIDS" + ) + error_list.append(error_substring) + if error_list: + if cli_output: + error_string = " and ".join(error_list) + error_string = f"Summary: {error_string}." + click.secho( + error_string, + bold=True, + fg="red", + ) + if not allow_errors: + return False + return True diff --git a/dandi/cli/cmd_validate.py b/dandi/cli/cmd_validate.py index 7229b5c96..64be0c1e8 100644 --- a/dandi/cli/cmd_validate.py +++ b/dandi/cli/cmd_validate.py @@ -25,18 +25,21 @@ def validate_bids( paths, schema=None, devel_debug=False, report=False, report_flag=False ): """Validate BIDS paths.""" + from ..bids_utils import evaluate_validation from ..validate import validate_bids as validate_bids_ if report_flag and not report: report = report_flag - validated = validate_bids_( + validator_result = validate_bids_( *paths, report=report, schema_version=schema, devel_debug=devel_debug, ) - if not validated: + valid = evaluate_validation(validator_result, cli_output=True) + + if not valid: raise SystemExit(1) diff --git a/dandi/upload.py b/dandi/upload.py index 8667e02cb..1c0a3250b 100644 --- a/dandi/upload.py +++ b/dandi/upload.py @@ -10,6 +10,7 @@ import click from . import lgr +from .bids_utils import evaluate_validation from .consts import DRAFT, dandiset_identifier_regex, dandiset_metadata_file from .dandiapi import RemoteAsset from .exceptions import NotFoundError @@ -452,8 +453,13 @@ def _bids_discover_and_validate( bids_datasets_to_validate.sort() validated_datasets = [] for bd in bids_datasets_to_validate: - validated = validate_bids(bd, allow_errors=validation == "ignore") - if validated: + validator_result = validate_bids(bd) + valid = evaluate_validation( + validator_result, + allow_errors=validation == "ignore", + cli_output=False, + ) + if valid: validated_datasets.append(bd) return validated_datasets else: diff --git a/dandi/validate.py b/dandi/validate.py index bd72f47d6..9f8cd5508 100644 --- a/dandi/validate.py +++ b/dandi/validate.py @@ -1,10 +1,7 @@ from pathlib import Path from typing import Iterator, List, Optional, Tuple, Union -import click - from .files import find_dandi_files -from .utils import pluralize # TODO: provide our own "errors" records, which would also include warnings etc @@ -14,8 +11,7 @@ def validate_bids( schema_version: Optional[str] = None, devel_debug: bool = False, report: Optional[str] = None, - allow_errors: Optional[bool] = False, -) -> bool: +) -> dict: """Validate BIDS paths. Parameters @@ -30,8 +26,6 @@ def validate_bids( If `True` a log will be written using the standard output path of `.write_report()`. If string, the string will be used as the output path. If the variable evaluates as False, no log will be written. - allow_errors : bool, optional - Whether to raise errors on invalid dataset. Returns ------- @@ -48,35 +42,7 @@ def validate_bids( validation_result = validate_bids_( paths, schema_version=schema_version, debug=devel_debug, report_path=report ) - missing_files = [ - pattern["regex"] - for pattern in validation_result["schema_tracking"] - if pattern["mandatory"] - ] - error_list = [] - if missing_files: - error_substring = ( - f"{pluralize(len(missing_files), 'filename pattern')} required " - "by BIDS could not be found" - ) - error_list.append(error_substring) - if validation_result["path_tracking"]: - error_substring = ( - f"{pluralize(len(validation_result['path_tracking']), 'filename')} " - "did not match any pattern known to BIDS" - ) - error_list.append(error_substring) - if error_list: - error_string = " and ".join(error_list) - error_string = f"Summary: {error_string}." - click.secho( - error_string, - bold=True, - fg="red", - ) - if not allow_errors: - return False - return True + return validation_result def validate( From 0492dbb9575cdc45c3c551eb4969607acfcee398 Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Sun, 26 Jun 2022 22:45:40 -0400 Subject: [PATCH 12/30] Not computing list if errors are suppressed --- dandi/bids_utils.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/dandi/bids_utils.py b/dandi/bids_utils.py index 466236b02..2f7786fbd 100644 --- a/dandi/bids_utils.py +++ b/dandi/bids_utils.py @@ -1,15 +1,13 @@ -from typing import Optional - -import click - from .utils import pluralize def evaluate_validation( validation_result: dict, - allow_errors: Optional[bool] = False, - cli_output: Optional[bool] = False, + allow_errors: bool = False, + cli_output: bool = False, ) -> bool: + if allow_errors: + return True missing_files = [ pattern["regex"] for pattern in validation_result["schema_tracking"] @@ -29,6 +27,8 @@ def evaluate_validation( ) error_list.append(error_substring) if error_list: + import click + if cli_output: error_string = " and ".join(error_list) error_string = f"Summary: {error_string}." @@ -37,6 +37,5 @@ def evaluate_validation( bold=True, fg="red", ) - if not allow_errors: - return False + return False return True From d079d9c0745bcb31ec7b4c620bc0d1bc03ffd6f0 Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Mon, 27 Jun 2022 00:13:38 -0400 Subject: [PATCH 13/30] Typing --- dandi/validate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dandi/validate.py b/dandi/validate.py index 9f8cd5508..8729d1b77 100644 --- a/dandi/validate.py +++ b/dandi/validate.py @@ -42,7 +42,7 @@ def validate_bids( validation_result = validate_bids_( paths, schema_version=schema_version, debug=devel_debug, report_path=report ) - return validation_result + return dict(validation_result) def validate( From 540923ab9805f2d816de9a76f4c458d83604c659 Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Thu, 30 Jun 2022 13:31:04 -0400 Subject: [PATCH 14/30] docstring fix --- dandi/validate.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dandi/validate.py b/dandi/validate.py index 8729d1b77..b0d857537 100644 --- a/dandi/validate.py +++ b/dandi/validate.py @@ -29,8 +29,9 @@ def validate_bids( Returns ------- - bool - Whether or not there were zero validation errors. + dict + Dictionary reporting required patterns not found and existing filenames not matching any + patterns. Notes ----- From a3b89dc14b90abfb75fbc7c25d87860f50afd196 Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Thu, 30 Jun 2022 14:49:32 -0400 Subject: [PATCH 15/30] Separating validation evaluation logic from reporting --- dandi/bids_utils.py | 66 ++++++++++++++++++++++++++++++--------- dandi/cli/cmd_validate.py | 5 +-- dandi/upload.py | 4 +-- 3 files changed, 56 insertions(+), 19 deletions(-) diff --git a/dandi/bids_utils.py b/dandi/bids_utils.py index 2f7786fbd..e5da7788c 100644 --- a/dandi/bids_utils.py +++ b/dandi/bids_utils.py @@ -3,11 +3,46 @@ def evaluate_validation( validation_result: dict, - allow_errors: bool = False, - cli_output: bool = False, + allow_invalid_filenames: bool = False, + allow_missing_files: bool = False, ) -> bool: - if allow_errors: + """Determine whether a dataset validation result marks it as valid. + + Parameters + ---------- + validation_result: dict + Dictionary as returned by `dandi.bids_validator_xs.validate_bids()`. + allow_missing_files: bool, optional + Whether to consider the dataset invalid if any mandatory files are not present. + allow_invalid_filenames: bool, optional + Whether to consider the dataset invalid if any filenames inside are invalid. + + Returns + ------- + bool: whether the dataset validation result marks it as valid. + + """ + + if allow_invalid_filenames and allow_missing_files: return True + missing_files = [ + i["regex"] for i in validation_result["schema_tracking"] if i["mandatory"] + ] + invalid_filenames = validation_result["path_tracking"] + + if missing_files and not allow_missing_files: + return False + if invalid_filenames and not allow_invalid_filenames: + return False + else: + return True + + +def report_errors( + validation_result: dict, +): + import click + missing_files = [ pattern["regex"] for pattern in validation_result["schema_tracking"] @@ -27,15 +62,16 @@ def evaluate_validation( ) error_list.append(error_substring) if error_list: - import click - - if cli_output: - error_string = " and ".join(error_list) - error_string = f"Summary: {error_string}." - click.secho( - error_string, - bold=True, - fg="red", - ) - return False - return True + error_string = " and ".join(error_list) + error_string = f"Summary: {error_string}." + click.secho( + error_string, + bold=True, + fg="red", + ) + else: + click.secho( + "All filenames are BIDS-valid and no mandatory files are missing.", + bold=True, + fg="green", + ) diff --git a/dandi/cli/cmd_validate.py b/dandi/cli/cmd_validate.py index 64be0c1e8..1a565fb22 100644 --- a/dandi/cli/cmd_validate.py +++ b/dandi/cli/cmd_validate.py @@ -25,7 +25,7 @@ def validate_bids( paths, schema=None, devel_debug=False, report=False, report_flag=False ): """Validate BIDS paths.""" - from ..bids_utils import evaluate_validation + from ..bids_utils import evaluate_validation, report_errors from ..validate import validate_bids as validate_bids_ if report_flag and not report: @@ -37,7 +37,8 @@ def validate_bids( schema_version=schema, devel_debug=devel_debug, ) - valid = evaluate_validation(validator_result, cli_output=True) + valid = evaluate_validation(validator_result) + report_errors(validator_result) if not valid: raise SystemExit(1) diff --git a/dandi/upload.py b/dandi/upload.py index 1c0a3250b..b9a0c2c89 100644 --- a/dandi/upload.py +++ b/dandi/upload.py @@ -456,8 +456,8 @@ def _bids_discover_and_validate( validator_result = validate_bids(bd) valid = evaluate_validation( validator_result, - allow_errors=validation == "ignore", - cli_output=False, + allow_missing_files=validation == "ignore", + allow_invalid_filenames=validation == "ignore", ) if valid: validated_datasets.append(bd) From c59bae2fdf40c0653abe03570723fdf2dced5cb4 Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Thu, 30 Jun 2022 14:57:07 -0400 Subject: [PATCH 16/30] fix typing --- dandi/bids_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dandi/bids_utils.py b/dandi/bids_utils.py index e5da7788c..06ac0d8a3 100644 --- a/dandi/bids_utils.py +++ b/dandi/bids_utils.py @@ -40,7 +40,7 @@ def evaluate_validation( def report_errors( validation_result: dict, -): +) -> None: import click missing_files = [ From 358d2f2dd7b47d25c2b90851209d5c8f48718763 Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Thu, 30 Jun 2022 16:16:18 -0400 Subject: [PATCH 17/30] Improve function naming --- dandi/bids_utils.py | 2 +- dandi/cli/cmd_validate.py | 4 ++-- dandi/upload.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/dandi/bids_utils.py b/dandi/bids_utils.py index 06ac0d8a3..3e4cdbe09 100644 --- a/dandi/bids_utils.py +++ b/dandi/bids_utils.py @@ -1,7 +1,7 @@ from .utils import pluralize -def evaluate_validation( +def is_valid( validation_result: dict, allow_invalid_filenames: bool = False, allow_missing_files: bool = False, diff --git a/dandi/cli/cmd_validate.py b/dandi/cli/cmd_validate.py index 1a565fb22..553e40a69 100644 --- a/dandi/cli/cmd_validate.py +++ b/dandi/cli/cmd_validate.py @@ -25,7 +25,7 @@ def validate_bids( paths, schema=None, devel_debug=False, report=False, report_flag=False ): """Validate BIDS paths.""" - from ..bids_utils import evaluate_validation, report_errors + from ..bids_utils import is_valid, report_errors from ..validate import validate_bids as validate_bids_ if report_flag and not report: @@ -37,7 +37,7 @@ def validate_bids( schema_version=schema, devel_debug=devel_debug, ) - valid = evaluate_validation(validator_result) + valid = is_valid(validator_result) report_errors(validator_result) if not valid: diff --git a/dandi/upload.py b/dandi/upload.py index b9a0c2c89..a009dba74 100644 --- a/dandi/upload.py +++ b/dandi/upload.py @@ -10,7 +10,7 @@ import click from . import lgr -from .bids_utils import evaluate_validation +from .bids_utils import is_valid from .consts import DRAFT, dandiset_identifier_regex, dandiset_metadata_file from .dandiapi import RemoteAsset from .exceptions import NotFoundError @@ -454,7 +454,7 @@ def _bids_discover_and_validate( validated_datasets = [] for bd in bids_datasets_to_validate: validator_result = validate_bids(bd) - valid = evaluate_validation( + valid = is_valid( validator_result, allow_missing_files=validation == "ignore", allow_invalid_filenames=validation == "ignore", From 62cf170cd0b5f435213c7707922641a6c34a2756 Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Mon, 4 Jul 2022 20:23:29 -0400 Subject: [PATCH 18/30] Testing data --- dandi/tests/fixtures.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/dandi/tests/fixtures.py b/dandi/tests/fixtures.py index 385f52de6..50c06885c 100644 --- a/dandi/tests/fixtures.py +++ b/dandi/tests/fixtures.py @@ -2,6 +2,7 @@ from dataclasses import dataclass, field from datetime import datetime +from distutils.dir_util import copy_tree import logging import os from pathlib import Path @@ -411,6 +412,15 @@ def zarr_dandiset(new_dandiset: SampleDandiset) -> SampleDandiset: return new_dandiset +@pytest.fixture() +def bids_example_dandiset( + new_dandiset: SampleDandiset, bids_examples: Callable[[], Iterator[str]] +) -> SampleDandiset: + + copy_tree(os.path.join(bids_examples, "asl003"), new_dandiset.dspath) + return new_dandiset + + @pytest.fixture() def video_files(tmp_path): video_paths = [] From 044ee28223819a0e5a5023aa6188dbea6fdc0ea0 Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Mon, 4 Jul 2022 21:08:36 -0400 Subject: [PATCH 19/30] Fixed typing --- dandi/tests/fixtures.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dandi/tests/fixtures.py b/dandi/tests/fixtures.py index 50c06885c..ef87e4f64 100644 --- a/dandi/tests/fixtures.py +++ b/dandi/tests/fixtures.py @@ -413,11 +413,11 @@ def zarr_dandiset(new_dandiset: SampleDandiset) -> SampleDandiset: @pytest.fixture() -def bids_example_dandiset( - new_dandiset: SampleDandiset, bids_examples: Callable[[], Iterator[str]] +def bids_dandiset( + new_dandiset: SampleDandiset, bids_examples: str ) -> SampleDandiset: - copy_tree(os.path.join(bids_examples, "asl003"), new_dandiset.dspath) + copy_tree(os.path.join(bids_examples, "asl003"), str(new_dandiset.dspath)) return new_dandiset From ac375332dfa4bb1aef1364d5a10ffd645665a191 Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Mon, 4 Jul 2022 21:08:44 -0400 Subject: [PATCH 20/30] testing bids upload --- dandi/tests/test_upload.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dandi/tests/test_upload.py b/dandi/tests/test_upload.py index e042024d2..fd0e5bbc9 100644 --- a/dandi/tests/test_upload.py +++ b/dandi/tests/test_upload.py @@ -181,6 +181,12 @@ def test_upload_sync_folder( text_dandiset.dandiset.get_asset_by_path("subdir2/banana.txt") +def test_upload_sync_bids(mocker, bids_dandiset): + confirm_mock = mocker.patch("click.confirm", return_value=True) + bids_dandiset.upload() + confirm_mock.assert_called_with("Delete 1 asset on server?") + + def test_upload_sync_zarr(mocker, zarr_dandiset): rmtree(zarr_dandiset.dspath / "sample.zarr") zarr.save(zarr_dandiset.dspath / "identity.zarr", np.eye(5)) From 306b2f9070a6df97faa4b28eab06a4d4323de53a Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Mon, 4 Jul 2022 21:13:21 -0400 Subject: [PATCH 21/30] using shutil instead of deprecated distutils --- dandi/tests/fixtures.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dandi/tests/fixtures.py b/dandi/tests/fixtures.py index ef87e4f64..3d819e7ae 100644 --- a/dandi/tests/fixtures.py +++ b/dandi/tests/fixtures.py @@ -2,7 +2,6 @@ from dataclasses import dataclass, field from datetime import datetime -from distutils.dir_util import copy_tree import logging import os from pathlib import Path @@ -417,7 +416,7 @@ def bids_dandiset( new_dandiset: SampleDandiset, bids_examples: str ) -> SampleDandiset: - copy_tree(os.path.join(bids_examples, "asl003"), str(new_dandiset.dspath)) + shutil.copytree(os.path.join(bids_examples, "asl003"), str(new_dandiset.dspath)) return new_dandiset From 4847cbb41a8fc60ee64a819b914daaad7213304b Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Tue, 5 Jul 2022 02:18:03 -0400 Subject: [PATCH 22/30] fixing copytree parameter --- dandi/tests/fixtures.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/dandi/tests/fixtures.py b/dandi/tests/fixtures.py index 3d819e7ae..7f747a9a2 100644 --- a/dandi/tests/fixtures.py +++ b/dandi/tests/fixtures.py @@ -412,11 +412,13 @@ def zarr_dandiset(new_dandiset: SampleDandiset) -> SampleDandiset: @pytest.fixture() -def bids_dandiset( - new_dandiset: SampleDandiset, bids_examples: str -) -> SampleDandiset: +def bids_dandiset(new_dandiset: SampleDandiset, bids_examples: str) -> SampleDandiset: - shutil.copytree(os.path.join(bids_examples, "asl003"), str(new_dandiset.dspath)) + shutil.copytree( + os.path.join(bids_examples, "asl003"), + os.path.join(str(new_dandiset.dspath), "asl003"), + copy_function=shutil.copy, + ) return new_dandiset From 9f82246ad26f570ce7db3e9f2ff2d05e86402b80 Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Tue, 5 Jul 2022 04:17:55 -0400 Subject: [PATCH 23/30] improved bids upload testing --- dandi/tests/fixtures.py | 1 - dandi/tests/test_upload.py | 8 ++++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/dandi/tests/fixtures.py b/dandi/tests/fixtures.py index 7f747a9a2..bad37bfe1 100644 --- a/dandi/tests/fixtures.py +++ b/dandi/tests/fixtures.py @@ -413,7 +413,6 @@ def zarr_dandiset(new_dandiset: SampleDandiset) -> SampleDandiset: @pytest.fixture() def bids_dandiset(new_dandiset: SampleDandiset, bids_examples: str) -> SampleDandiset: - shutil.copytree( os.path.join(bids_examples, "asl003"), os.path.join(str(new_dandiset.dspath), "asl003"), diff --git a/dandi/tests/test_upload.py b/dandi/tests/test_upload.py index fd0e5bbc9..3bab8ecec 100644 --- a/dandi/tests/test_upload.py +++ b/dandi/tests/test_upload.py @@ -181,10 +181,10 @@ def test_upload_sync_folder( text_dandiset.dandiset.get_asset_by_path("subdir2/banana.txt") -def test_upload_sync_bids(mocker, bids_dandiset): - confirm_mock = mocker.patch("click.confirm", return_value=True) - bids_dandiset.upload() - confirm_mock.assert_called_with("Delete 1 asset on server?") +def test_upload_bids(mocker: MockerFixture, bids_dandiset: SampleDandiset) -> None: + iter_upload_spy = mocker.spy(LocalFileAsset, "iter_upload") + bids_dandiset.upload(existing="forced") + iter_upload_spy.assert_called() def test_upload_sync_zarr(mocker, zarr_dandiset): From c230d268595e7703867c273936f4f883ed4029ca Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Tue, 5 Jul 2022 13:54:19 -0400 Subject: [PATCH 24/30] Creating changes file --- dandi/tests/fixtures.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dandi/tests/fixtures.py b/dandi/tests/fixtures.py index bad37bfe1..ac65e299f 100644 --- a/dandi/tests/fixtures.py +++ b/dandi/tests/fixtures.py @@ -415,9 +415,10 @@ def zarr_dandiset(new_dandiset: SampleDandiset) -> SampleDandiset: def bids_dandiset(new_dandiset: SampleDandiset, bids_examples: str) -> SampleDandiset: shutil.copytree( os.path.join(bids_examples, "asl003"), - os.path.join(str(new_dandiset.dspath), "asl003"), + str(new_dandiset.dspath) + "/", copy_function=shutil.copy, ) + (new_dandiset.dspath / "CHANGES").write_text("0.1.0 2014-11-03\n") return new_dandiset From bf3aba734d69a2a96ee46ad1f6492aaeaa4c5774 Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Tue, 5 Jul 2022 14:59:15 -0400 Subject: [PATCH 25/30] allowing existing directory --- dandi/tests/fixtures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dandi/tests/fixtures.py b/dandi/tests/fixtures.py index ac65e299f..38bd97acd 100644 --- a/dandi/tests/fixtures.py +++ b/dandi/tests/fixtures.py @@ -416,7 +416,7 @@ def bids_dandiset(new_dandiset: SampleDandiset, bids_examples: str) -> SampleDan shutil.copytree( os.path.join(bids_examples, "asl003"), str(new_dandiset.dspath) + "/", - copy_function=shutil.copy, + dirs_exist_ok=True, ) (new_dandiset.dspath / "CHANGES").write_text("0.1.0 2014-11-03\n") return new_dandiset From 7f58c0546927574e8d64c72631bf94d504d8ce69 Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Tue, 5 Jul 2022 15:29:50 -0400 Subject: [PATCH 26/30] boilerplate copytree --- dandi/tests/fixtures.py | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/dandi/tests/fixtures.py b/dandi/tests/fixtures.py index 38bd97acd..6cec12f8f 100644 --- a/dandi/tests/fixtures.py +++ b/dandi/tests/fixtures.py @@ -40,6 +40,32 @@ lgr = get_logger() +def copytree(src, dst, symlinks=False, ignore=None): + """Function mimicking `shutil.copytree()` behaviour but supporting existing target + directories. + + Notes + ----- + * This function can be removed and replaced by a call to `shutil.copytree()` + setting the `dirs_exist_ok` keyword argument to true, whenever Python 3.7 + is no longer supported. + + References + ---------- + https://docs.python.org/3/whatsnew/3.8.html#shutil + """ + if not os.path.exists(dst): + os.makedirs(dst) + for item in os.listdir(src): + s = os.path.join(src, item) + d = os.path.join(dst, item) + if os.path.isdir(s): + copytree(s, d, symlinks, ignore) + else: + if not os.path.exists(d) or os.stat(s).st_mtime - os.stat(d).st_mtime > 1: + shutil.copy2(s, d) + + @pytest.fixture(autouse=True) def capture_all_logs(caplog: pytest.LogCaptureFixture) -> None: caplog.set_level(logging.DEBUG, logger="dandi") @@ -413,10 +439,9 @@ def zarr_dandiset(new_dandiset: SampleDandiset) -> SampleDandiset: @pytest.fixture() def bids_dandiset(new_dandiset: SampleDandiset, bids_examples: str) -> SampleDandiset: - shutil.copytree( + copytree( os.path.join(bids_examples, "asl003"), str(new_dandiset.dspath) + "/", - dirs_exist_ok=True, ) (new_dandiset.dspath / "CHANGES").write_text("0.1.0 2014-11-03\n") return new_dandiset From 50092eaf1259ff16d9d5bc07c5e5a33d230c7a36 Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Wed, 6 Jul 2022 09:45:10 -0400 Subject: [PATCH 27/30] Check asset upload --- dandi/tests/test_upload.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/dandi/tests/test_upload.py b/dandi/tests/test_upload.py index 3bab8ecec..59c8ec8e9 100644 --- a/dandi/tests/test_upload.py +++ b/dandi/tests/test_upload.py @@ -184,7 +184,16 @@ def test_upload_sync_folder( def test_upload_bids(mocker: MockerFixture, bids_dandiset: SampleDandiset) -> None: iter_upload_spy = mocker.spy(LocalFileAsset, "iter_upload") bids_dandiset.upload(existing="forced") + # Check whether upload was run iter_upload_spy.assert_called() + # Check existence of assets: + dandiset = bids_dandiset.dandiset + # file we created? + dandiset.get_asset_by_path("CHANGES") + # BIDS descriptor file? + dandiset.get_asset_by_path("dataset_description.json") + # actual data file? + dandiset.get_asset_by_path("sub-Sub1/anat/sub-Sub1_T1w.nii.gz") def test_upload_sync_zarr(mocker, zarr_dandiset): From 427a136d5f13f1d45e207fcefb8257b99ef5950f Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Wed, 6 Jul 2022 15:12:42 -0400 Subject: [PATCH 28/30] Testing upload with validation ignore --- dandi/tests/test_upload.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/dandi/tests/test_upload.py b/dandi/tests/test_upload.py index 59c8ec8e9..d925db0a3 100644 --- a/dandi/tests/test_upload.py +++ b/dandi/tests/test_upload.py @@ -196,6 +196,23 @@ def test_upload_bids(mocker: MockerFixture, bids_dandiset: SampleDandiset) -> No dandiset.get_asset_by_path("sub-Sub1/anat/sub-Sub1_T1w.nii.gz") +def test_upload_bids_validation_ignore( + mocker: MockerFixture, bids_dandiset: SampleDandiset +) -> None: + iter_upload_spy = mocker.spy(LocalFileAsset, "iter_upload") + bids_dandiset.upload(existing="forced", validation="ignore") + # Check whether upload was run + iter_upload_spy.assert_called() + # Check existence of assets: + dandiset = bids_dandiset.dandiset + # file we created? + dandiset.get_asset_by_path("CHANGES") + # BIDS descriptor file? + dandiset.get_asset_by_path("dataset_description.json") + # actual data file? + dandiset.get_asset_by_path("sub-Sub1/anat/sub-Sub1_T1w.nii.gz") + + def test_upload_sync_zarr(mocker, zarr_dandiset): rmtree(zarr_dandiset.dspath / "sample.zarr") zarr.save(zarr_dandiset.dspath / "identity.zarr", np.eye(5)) From 2864c77148953d1a3c2a4597cb191962098662a7 Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Wed, 6 Jul 2022 15:15:58 -0400 Subject: [PATCH 29/30] simplified path processing in BIDS lookup --- dandi/upload.py | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/dandi/upload.py b/dandi/upload.py index a009dba74..f79369067 100644 --- a/dandi/upload.py +++ b/dandi/upload.py @@ -62,9 +62,8 @@ def upload( " paths. Use 'dandi download' or 'organize' first." ) - # pre-validate BIDS datasets before going for individual - # files etc - bids_datasets = _bids_discover_and_validate(dandiset_.path, paths, validation) + # Pre-validate BIDS datasets before going for individual files. + bids_datasets = _bids_discover_and_validate(dandiset_.path, validation) if bids_datasets: _bids_datasets = [str(i) for i in bids_datasets] @@ -403,7 +402,6 @@ def skip_file(msg: Any) -> Dict[str, str]: def _bids_discover_and_validate( dandiset_path: str, - paths: Optional[List[Union[str, Path]]] = None, validation: Optional[str] = "require", ) -> List[Path]: """Temporary implementation for discovery and validation of BIDS datasets @@ -415,10 +413,6 @@ def _bids_discover_and_validate( from .utils import find_files from .validate import validate_bids - if paths: - bids_lookup_paths = set(paths) - else: - bids_lookup_paths = None bids_descriptions = map( Path, find_files(r"(^|[/\x5C])dataset_description\.json$", dandiset_path) ) @@ -431,11 +425,9 @@ def _bids_discover_and_validate( ) if validation != "skip": - if bids_lookup_paths: + if bids_datasets: bids_datasets_to_validate = list() - for p in bids_lookup_paths: - if isinstance(p, str): - p = Path(p) + for p in bids_datasets: for bd in bids_datasets: try: p.relative_to(bd) From 8139966db2f801d4016599ee3b9a8571ea2c1596 Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Wed, 6 Jul 2022 16:07:12 -0400 Subject: [PATCH 30/30] docstring formatting --- dandi/tests/fixtures.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dandi/tests/fixtures.py b/dandi/tests/fixtures.py index 6cec12f8f..7a8b21998 100644 --- a/dandi/tests/fixtures.py +++ b/dandi/tests/fixtures.py @@ -47,8 +47,8 @@ def copytree(src, dst, symlinks=False, ignore=None): Notes ----- * This function can be removed and replaced by a call to `shutil.copytree()` - setting the `dirs_exist_ok` keyword argument to true, whenever Python 3.7 - is no longer supported. + setting the `dirs_exist_ok` keyword argument to true, whenever Python 3.7 + is no longer supported. References ----------