diff --git a/dandi/cli/tests/test_cmd_ls.py b/dandi/cli/tests/test_cmd_ls.py index 13dbde12d..eec7e4c7c 100644 --- a/dandi/cli/tests/test_cmd_ls.py +++ b/dandi/cli/tests/test_cmd_ls.py @@ -1,5 +1,8 @@ +from __future__ import annotations + import json -import os +from pathlib import Path +from typing import Any from unittest.mock import ANY from click.testing import CliRunner @@ -15,25 +18,30 @@ @pytest.mark.parametrize( "format", ("auto", "json", "json_pp", "json_lines", "yaml", "pyout") ) -def test_smoke(simple1_nwb_metadata, simple1_nwb, format): +def test_smoke( + simple1_nwb_metadata: dict[str, Any], simple1_nwb: Path, format: str +) -> None: runner = CliRunner() - r = runner.invoke(ls, ["-f", format, simple1_nwb]) + r = runner.invoke(ls, ["-f", format, str(simple1_nwb)]) assert r.exit_code == 0, f"Exited abnormally. out={r.stdout}" # we would need to redirect pyout for its analysis out = r.stdout if format == "json_lines": - load = json.loads + + def load(s: str) -> Any: + return json.loads(s) + elif format.startswith("json"): - def load(s): + def load(s: str) -> Any: obj = json.loads(s) assert len(obj) == 1 # will be a list with a single elem return obj[0] elif format == "yaml": - def load(s): + def load(s: str) -> Any: obj = yaml_load(s, typ="base") assert len(obj) == 1 # will be a list with a single elem return obj[0] @@ -49,20 +57,20 @@ def load(s): assert metadata[f] == simple1_nwb_metadata[f] -def test_ls_nwb_file(simple2_nwb): - bids_file_path = "simple2.nwb" - bids_file_path = os.path.join(simple2_nwb, bids_file_path) - r = CliRunner().invoke(ls, ["-f", "yaml", bids_file_path]) +def test_ls_nwb_file(simple2_nwb: Path) -> None: + bids_file_path = simple2_nwb / "simple2.nwb" + r = CliRunner().invoke(ls, ["-f", "yaml", str(bids_file_path)]) assert r.exit_code == 0, r.output data = yaml_load(r.stdout, "safe") assert len(data) == 1 @mark.skipif_no_network -def test_ls_bids_file(bids_examples): - bids_file_path = "asl003/sub-Sub1/anat/sub-Sub1_T1w.nii.gz" - bids_file_path = os.path.join(bids_examples, bids_file_path) - r = CliRunner().invoke(ls, ["-f", "yaml", bids_file_path]) +def test_ls_bids_file(bids_examples: Path) -> None: + bids_file_path = ( + bids_examples / "asl003" / "sub-Sub1" / "anat" / "sub-Sub1_T1w.nii.gz" + ) + r = CliRunner().invoke(ls, ["-f", "yaml", str(bids_file_path)]) assert r.exit_code == 0, r.output data = yaml_load(r.stdout, "safe") assert len(data) == 1 @@ -70,12 +78,16 @@ def test_ls_bids_file(bids_examples): @mark.skipif_no_network -def test_ls_zarrbids_file(bids_examples): +def test_ls_zarrbids_file(bids_examples: Path) -> None: bids_file_path = ( - "micr_SEMzarr/sub-01/ses-01/micr/sub-01_ses-01_sample-A_SPIM.ome.zarr" + bids_examples + / "micr_SEMzarr" + / "sub-01" + / "ses-01" + / "micr" + / "sub-01_ses-01_sample-A_SPIM.ome.zarr" ) - bids_file_path = os.path.join(bids_examples, bids_file_path) - r = CliRunner().invoke(ls, ["-f", "yaml", bids_file_path]) + r = CliRunner().invoke(ls, ["-f", "yaml", str(bids_file_path)]) assert r.exit_code == 0, r.output data = yaml_load(r.stdout, "safe") assert len(data) == 1 @@ -83,7 +95,7 @@ def test_ls_zarrbids_file(bids_examples): @mark.skipif_no_network -def test_ls_dandiset_url(): +def test_ls_dandiset_url() -> None: r = CliRunner().invoke( ls, ["-f", "yaml", "https://api.dandiarchive.org/api/dandisets/000027"] ) @@ -94,7 +106,7 @@ def test_ls_dandiset_url(): @mark.skipif_no_network -def test_ls_dandiset_url_recursive(): +def test_ls_dandiset_url_recursive() -> None: r = CliRunner().invoke( ls, ["-f", "yaml", "-r", "https://api.dandiarchive.org/api/dandisets/000027"] ) @@ -106,7 +118,7 @@ def test_ls_dandiset_url_recursive(): @mark.skipif_no_network -def test_ls_path_url(): +def test_ls_path_url() -> None: r = CliRunner().invoke( ls, [ @@ -124,7 +136,7 @@ def test_ls_path_url(): assert data[0]["path"] == "sub-RAT123/sub-RAT123.nwb" -def test_smoke_local_schema(simple1_nwb): +def test_smoke_local_schema(simple1_nwb: Path) -> None: runner = CliRunner() r = runner.invoke( ls, @@ -133,7 +145,7 @@ def test_smoke_local_schema(simple1_nwb): "json", "--schema", DANDI_SCHEMA_VERSION, - simple1_nwb, + str(simple1_nwb), ], ) assert r.exit_code == 0, f"Exited abnormally. out={r.stdout}" diff --git a/dandi/cli/tests/test_cmd_validate.py b/dandi/cli/tests/test_cmd_validate.py index 234cbed23..afa430a00 100644 --- a/dandi/cli/tests/test_cmd_validate.py +++ b/dandi/cli/tests/test_cmd_validate.py @@ -1,5 +1,4 @@ import json -import os from pathlib import Path from click.testing import CliRunner @@ -10,44 +9,40 @@ @pytest.mark.parametrize("dataset", BIDS_ERROR_TESTDATA_SELECTION) -def test_validate_bids_error(bids_error_examples, dataset): - - broken_dataset = os.path.join(bids_error_examples, dataset) - with open(os.path.join(broken_dataset, ".ERRORS.json")) as f: +def test_validate_bids_error(bids_error_examples: Path, dataset: str) -> None: + broken_dataset = bids_error_examples / dataset + with (broken_dataset / ".ERRORS.json").open() as f: expected_errors = json.load(f) - r = CliRunner().invoke(validate, [broken_dataset]) + r = CliRunner().invoke(validate, [str(broken_dataset)]) # Does it break? assert r.exit_code == 1 - # Does it detect all errors? for key in expected_errors: assert key in r.output -def test_validate_nwb_error(simple3_nwb): - """ - Do we fail on critical NWB validation errors? - """ - - r = CliRunner().invoke(validate, [simple3_nwb]) +def test_validate_nwb_error(simple3_nwb: Path) -> None: + """Do we fail on critical NWB validation errors?""" + r = CliRunner().invoke(validate, [str(simple3_nwb)]) # does it fail? as per: # https://github.com/dandi/dandi-cli/pull/1157#issuecomment-1312546812 assert r.exit_code != 0 -def test_validate_bids_grouping_error(bids_error_examples, dataset="invalid_asl003"): +def test_validate_bids_grouping_error( + bids_error_examples: Path, dataset: str = "invalid_asl003" +) -> None: """ - This is currently a placeholder test, and should be updated once we have paths with - multiple errors for which grouping functionality can actually be tested. + This is currently a placeholder test, and should be updated once we have + paths with multiple errors for which grouping functionality can actually be + tested. """ - - dataset = os.path.join(bids_error_examples, dataset) - r = CliRunner().invoke(validate, ["--grouping=path", dataset]) + bids_dataset = bids_error_examples / dataset + r = CliRunner().invoke(validate, ["--grouping=path", str(bids_dataset)]) # Does it break? assert r.exit_code == 1 - # Does it detect all errors? - assert dataset in r.output + assert str(bids_dataset) in r.output def test_validate_nwb_path_grouping(organized_nwb_dir3: Path) -> None: @@ -64,15 +59,13 @@ def test_validate_nwb_path_grouping(organized_nwb_dir3: Path) -> None: def test_validate_bids_error_grouping_notification( - bids_error_examples, dataset="invalid_asl003" -): + bids_error_examples: Path, dataset: str = "invalid_asl003" +) -> None: """Test user notification for unimplemented parameter value.""" - - broken_dataset = os.path.join(bids_error_examples, dataset) - r = CliRunner().invoke(validate, ["--grouping=error", broken_dataset]) + broken_dataset = bids_error_examples / dataset + r = CliRunner().invoke(validate, ["--grouping=error", str(broken_dataset)]) # Does it break? assert r.exit_code == 2 - # Does it notify the user correctly? notification_substring = "Invalid value for '--grouping'" assert notification_substring in r.output diff --git a/dandi/tests/fixtures.py b/dandi/tests/fixtures.py index e0b473d4c..1184b159f 100644 --- a/dandi/tests/fixtures.py +++ b/dandi/tests/fixtures.py @@ -55,9 +55,10 @@ BIDS_ERROR_TESTDATA_SELECTION = ["invalid_asl003", "invalid_pet001"] -def copytree(src, dst, symlinks=False, ignore=None): - """Function mimicking `shutil.copytree()` behaviour but supporting existing target - directories. +def copytree(src: str | Path, dst: str | Path) -> None: + """ + Function mimicking `shutil.copytree()` behaviour but supporting existing + target directories. Notes ----- @@ -75,7 +76,7 @@ def copytree(src, dst, symlinks=False, ignore=None): s = os.path.join(src, item) d = os.path.join(dst, item) if os.path.isdir(s): - copytree(s, d, symlinks, ignore) + copytree(s, d) else: if not os.path.exists(d) or os.stat(s).st_mtime - os.stat(d).st_mtime > 1: shutil.copy2(s, d) @@ -112,9 +113,9 @@ def simple1_nwb_metadata() -> Dict[str, Any]: @pytest.fixture(scope="session") def simple1_nwb( simple1_nwb_metadata: Dict[str, Any], tmp_path_factory: pytest.TempPathFactory -) -> str: +) -> Path: return make_nwb_file( - str(tmp_path_factory.mktemp("simple1") / "simple1.nwb"), + tmp_path_factory.mktemp("simple1") / "simple1.nwb", **simple1_nwb_metadata, ) @@ -122,10 +123,10 @@ def simple1_nwb( @pytest.fixture(scope="session") def simple2_nwb( simple1_nwb_metadata: Dict[str, Any], tmp_path_factory: pytest.TempPathFactory -) -> str: +) -> Path: """With a subject""" return make_nwb_file( - str(tmp_path_factory.mktemp("simple2") / "simple2.nwb"), + tmp_path_factory.mktemp("simple2") / "simple2.nwb", subject=pynwb.file.Subject( subject_id="mouse001", date_of_birth=datetime(2016, 12, 1, tzinfo=tzutc()), @@ -139,10 +140,10 @@ def simple2_nwb( @pytest.fixture(scope="session") def simple3_nwb( simple1_nwb_metadata: Dict[str, Any], tmp_path_factory: pytest.TempPathFactory -) -> str: +) -> Path: """With a subject, but no subject_id.""" return make_nwb_file( - str(tmp_path_factory.mktemp("simple3") / "simple3.nwb"), + tmp_path_factory.mktemp("simple3") / "simple3.nwb", subject=pynwb.file.Subject( age="P1D/", sex="O", @@ -153,7 +154,7 @@ def simple3_nwb( @pytest.fixture(scope="session") -def simple4_nwb(tmp_path_factory: pytest.TempPathFactory) -> str: +def simple4_nwb(tmp_path_factory: pytest.TempPathFactory) -> Path: """ With subject, subject_id, species, but including data orientation ambiguity, the only currently non-critical issue in the dandi schema for nwbinspector validation: @@ -183,7 +184,7 @@ def simple4_nwb(tmp_path_factory: pytest.TempPathFactory) -> str: ), ) nwbfile.add_acquisition(time_series) - filename = str(tmp_path_factory.mktemp("simple4") / "simple4.nwb") + filename = tmp_path_factory.mktemp("simple4") / "simple4.nwb" with pynwb.NWBHDF5IO(filename, "w") as io: io.write(nwbfile, cache_spec=False) return filename @@ -191,7 +192,7 @@ def simple4_nwb(tmp_path_factory: pytest.TempPathFactory) -> str: @pytest.fixture(scope="session") def organized_nwb_dir( - simple2_nwb: str, tmp_path_factory: pytest.TempPathFactory + simple2_nwb: Path, tmp_path_factory: pytest.TempPathFactory ) -> Path: tmp_path = tmp_path_factory.mktemp("organized_nwb_dir") (tmp_path / dandiset_metadata_file).write_text("{}\n") @@ -205,7 +206,7 @@ def organized_nwb_dir( @pytest.fixture(scope="session") def organized_nwb_dir2( simple1_nwb_metadata: Dict[str, Any], - simple2_nwb: str, + simple2_nwb: Path, tmp_path_factory: pytest.TempPathFactory, ) -> Path: tmp_path = tmp_path_factory.mktemp("organized_nwb_dir2") @@ -214,7 +215,7 @@ def organized_nwb_dir2( # file to be "organized" shutil.copy(simple2_nwb, tmp_path) make_nwb_file( - str(tmp_path / "simple3.nwb"), + tmp_path / "simple3.nwb", subject=pynwb.file.Subject( subject_id="lizard001", date_of_birth=datetime(2016, 12, 1, tzinfo=tzutc()), @@ -232,7 +233,7 @@ def organized_nwb_dir2( @pytest.fixture(scope="session") def organized_nwb_dir3( - simple4_nwb: str, tmp_path_factory: pytest.TempPathFactory + simple4_nwb: Path, tmp_path_factory: pytest.TempPathFactory ) -> Path: tmp_path = tmp_path_factory.mktemp("organized_nwb_dir") (tmp_path / dandiset_metadata_file).write_text("{}\n") @@ -260,18 +261,18 @@ def get_gitrepo_fixture( committish: Optional[str] = None, scope: Scope = "session", make_subdirs_dandisets: bool = False, -) -> Callable[[pytest.TempPathFactory], str]: +) -> Callable[[pytest.TempPathFactory], Path]: if committish: raise NotImplementedError() @pytest.fixture(scope=scope) - def fixture(tmp_path_factory: pytest.TempPathFactory) -> str: + def fixture(tmp_path_factory: pytest.TempPathFactory) -> Path: skipif.no_network() skipif.no_git() - path = str(tmp_path_factory.mktemp("gitrepo")) + path = tmp_path_factory.mktemp("gitrepo") lgr.debug("Cloning %r into %r", url, path) - run(["git", "clone", "--depth=1", url, path], check=True) + run(["git", "clone", "--depth=1", url, str(path)], check=True) if make_subdirs_dandisets: _make_subdirs_dandisets(path) return path @@ -283,14 +284,14 @@ def get_filtered_gitrepo_fixture( url: str, whitelist: List[str], make_subdirs_dandisets: Optional[bool] = False, -) -> Callable[[pytest.TempPathFactory], Iterator[str]]: +) -> Callable[[pytest.TempPathFactory], Iterator[Path]]: @pytest.fixture(scope="session") def fixture( tmp_path_factory: pytest.TempPathFactory, - ) -> Iterator[str]: + ) -> Iterator[Path]: skipif.no_network() skipif.no_git() - path = str(tmp_path_factory.mktemp("gitrepo")) + path = tmp_path_factory.mktemp("gitrepo") lgr.debug("Cloning %r into %r", url, path) run( [ @@ -300,15 +301,15 @@ def fixture( "--filter=blob:none", "--sparse", url, - path, + str(path), ], check=True, ) # cwd specification is VERY important, not only to achieve the correct # effects, but also to avoid dropping files from your repository if you # were to run `git sparse-checkout` inside the software repo. - run(["git", "sparse-checkout", "init", "--cone"], cwd=path, check=True) - run(["git", "sparse-checkout", "set"] + whitelist, cwd=path, check=True) + run(["git", "sparse-checkout", "init", "--cone"], cwd=str(path), check=True) + run(["git", "sparse-checkout", "set"] + whitelist, cwd=str(path), check=True) if make_subdirs_dandisets: _make_subdirs_dandisets(path) yield path @@ -316,9 +317,8 @@ def fixture( return fixture -def _make_subdirs_dandisets(path: str) -> None: - for i in os.listdir(path): - bids_dataset_path = Path(path) / i +def _make_subdirs_dandisets(path: Path) -> None: + for bids_dataset_path in path.iterdir(): if bids_dataset_path.is_dir(): (bids_dataset_path / dandiset_metadata_file).write_text(" \n") @@ -547,45 +547,36 @@ def zarr_dandiset(new_dandiset: SampleDandiset) -> SampleDandiset: @pytest.fixture() -def bids_dandiset(new_dandiset: SampleDandiset, bids_examples: str) -> SampleDandiset: - copytree( - os.path.join(bids_examples, "asl003"), - str(new_dandiset.dspath) + "/", - ) +def bids_dandiset(new_dandiset: SampleDandiset, bids_examples: Path) -> SampleDandiset: + copytree(bids_examples / "asl003", new_dandiset.dspath) (new_dandiset.dspath / "CHANGES").write_text("0.1.0 2014-11-03\n") return new_dandiset @pytest.fixture() def bids_nwb_dandiset( - new_dandiset: SampleDandiset, bids_examples: str + new_dandiset: SampleDandiset, bids_examples: Path ) -> SampleDandiset: - copytree( - os.path.join(bids_examples, "ieeg_epilepsyNWB"), - str(new_dandiset.dspath) + "/", - ) + copytree(bids_examples / "ieeg_epilepsyNWB", new_dandiset.dspath) (new_dandiset.dspath / "CHANGES").write_text("0.1.0 2014-11-03\n") return new_dandiset @pytest.fixture() def bids_dandiset_invalid( - new_dandiset: SampleDandiset, bids_error_examples: str + new_dandiset: SampleDandiset, bids_error_examples: Path ) -> SampleDandiset: dataset_path = new_dandiset.dspath - copytree( - os.path.join(bids_error_examples, "invalid_pet001"), - str(dataset_path) + "/", - ) - os.remove(os.path.join(dataset_path, "README")) + copytree(bids_error_examples / "invalid_pet001", dataset_path) + (dataset_path / "README").unlink() return new_dandiset @pytest.fixture() -def video_files(tmp_path): - video_paths = [] +def video_files(tmp_path: Path) -> list[tuple[Path, Path]]: import cv2 + video_paths = [] video_path = tmp_path / "video_files" video_path.mkdir() for no in range(2): diff --git a/dandi/tests/test_dandiapi.py b/dandi/tests/test_dandiapi.py index 8e2580063..09717d7b9 100644 --- a/dandi/tests/test_dandiapi.py +++ b/dandi/tests/test_dandiapi.py @@ -1,7 +1,6 @@ import builtins from datetime import datetime, timezone import logging -import os.path from pathlib import Path import random import re @@ -32,7 +31,9 @@ from ..utils import list_paths -def test_upload(new_dandiset: SampleDandiset, simple1_nwb: str, tmp_path: Path) -> None: +def test_upload( + new_dandiset: SampleDandiset, simple1_nwb: Path, tmp_path: Path +) -> None: d = new_dandiset.dandiset assert d.version_id == DRAFT d.upload_raw_asset(simple1_nwb, {"path": "testing/simple1.nwb"}) @@ -41,7 +42,7 @@ def test_upload(new_dandiset: SampleDandiset, simple1_nwb: str, tmp_path: Path) d.download_directory("", tmp_path) paths = list_paths(tmp_path) assert paths == [tmp_path / "testing" / "simple1.nwb"] - assert paths[0].stat().st_size == os.path.getsize(simple1_nwb) + assert paths[0].stat().st_size == simple1_nwb.stat().st_size def test_publish_and_manipulate(new_dandiset: SampleDandiset, tmp_path: Path) -> None: @@ -92,7 +93,7 @@ def test_publish_and_manipulate(new_dandiset: SampleDandiset, tmp_path: Path) -> ) -def test_get_asset_metadata(new_dandiset: SampleDandiset, simple1_nwb: str) -> None: +def test_get_asset_metadata(new_dandiset: SampleDandiset, simple1_nwb: Path) -> None: d = new_dandiset.dandiset d.upload_raw_asset(simple1_nwb, {"path": "testing/simple1.nwb", "foo": "bar"}) (asset,) = d.get_assets() diff --git a/dandi/tests/test_download.py b/dandi/tests/test_download.py index 7570b4ecd..21d62a5bc 100644 --- a/dandi/tests/test_download.py +++ b/dandi/tests/test_download.py @@ -1,6 +1,5 @@ import json import os -import os.path as op from pathlib import Path import re from shutil import rmtree @@ -123,12 +122,10 @@ def test_download_000027_resume( with (dldir / "checksum").open("w") as fp: json.dump(digests, fp) download(url, tmp_path, get_metadata=False) - contents = [ - op.relpath(op.join(dirpath, entry), dsdir) - for (dirpath, dirnames, filenames) in os.walk(dsdir) - for entry in dirnames + filenames + assert list_paths(dsdir, dirs=True) == [ + dsdir / "sub-RAT123", + dsdir / "sub-RAT123" / "sub-RAT123.nwb", ] - assert sorted(contents) == ["sub-RAT123", op.join("sub-RAT123", "sub-RAT123.nwb")] assert nwb.stat().st_size == size assert digester(str(nwb)) == digests diff --git a/dandi/tests/test_files.py b/dandi/tests/test_files.py index 99b50ea03..7e0ad2aed 100644 --- a/dandi/tests/test_files.py +++ b/dandi/tests/test_files.py @@ -310,7 +310,7 @@ def test_dandi_file_zarr_with_excluded_dotfiles(tmp_path: Path) -> None: assert isinstance(zf, ZarrAsset) -def test_validate_simple1(simple1_nwb): +def test_validate_simple1(simple1_nwb: Path) -> None: # this file should be ok as long as schema_version is specified errors = dandi_file(simple1_nwb).get_validation_errors( schema_version=get_schema_version() @@ -318,9 +318,14 @@ def test_validate_simple1(simple1_nwb): assert [e.message for e in errors] == ["File is not inside a Dandiset"] -def test_validate_simple1_no_subject(simple1_nwb): +def test_validate_simple1_no_subject(simple1_nwb: Path) -> None: errors = dandi_file(simple1_nwb).get_validation_errors() - assert sorted(e.message for e in errors) == [ + errmsgs = [] + for e in errors: + assert e.message is not None + errmsgs.append(e.message) + errmsgs.sort() + assert errmsgs == [ "File is not inside a Dandiset", "Subject is missing.", ] @@ -344,9 +349,14 @@ def test_validate_simple2_new(organized_nwb_dir: Path) -> None: assert not errors -def test_validate_simple3_no_subject_id(simple3_nwb): +def test_validate_simple3_no_subject_id(simple3_nwb: Path) -> None: errors = dandi_file(simple3_nwb).get_validation_errors() - assert sorted(e.message for e in errors) == [ + errmsgs = [] + for e in errors: + assert e.message is not None + errmsgs.append(e.message) + errmsgs.sort() + assert errmsgs == [ "File is not inside a Dandiset", "subject_id is missing.", ] diff --git a/dandi/tests/test_metadata.py b/dandi/tests/test_metadata.py index f02e2666a..2bd14c4bb 100644 --- a/dandi/tests/test_metadata.py +++ b/dandi/tests/test_metadata.py @@ -1,6 +1,5 @@ from datetime import datetime, timedelta import json -import os from pathlib import Path import shutil from shutil import copyfile @@ -53,7 +52,7 @@ METADATA_DIR = Path(__file__).with_name("data") / "metadata" -def test_get_metadata(simple1_nwb: str, simple1_nwb_metadata: Dict[str, Any]) -> None: +def test_get_metadata(simple1_nwb: Path, simple1_nwb_metadata: Dict[str, Any]) -> None: target_metadata = simple1_nwb_metadata.copy() # we will also get some counts target_metadata["number_of_electrodes"] = 0 @@ -65,7 +64,7 @@ def test_get_metadata(simple1_nwb: str, simple1_nwb_metadata: Dict[str, Any]) -> # we do not populate any subject fields in our simple1_nwb for f in metadata_nwb_subject_fields: target_metadata[f] = None - metadata = get_metadata(str(simple1_nwb)) + metadata = get_metadata(simple1_nwb) # we also load nwb_version field, so it must not be degenerate and ATM # it is 2.X.Y. And since I don't know how to query pynwb on what # version it currently "supports", we will just pop it @@ -73,7 +72,7 @@ def test_get_metadata(simple1_nwb: str, simple1_nwb_metadata: Dict[str, Any]) -> assert target_metadata == metadata -def test_bids_nwb_metadata_integration(bids_examples, tmp_path): +def test_bids_nwb_metadata_integration(bids_examples: Path, tmp_path: Path) -> None: """ Notes ----- @@ -82,15 +81,16 @@ def test_bids_nwb_metadata_integration(bids_examples, tmp_path): https://github.com/dandi/dandi-cli/pull/1183#discussion_r1061622910 """ - source_dpath = os.path.join(bids_examples, "ieeg_epilepsyNWB") - dpath = os.path.join(tmp_path, "ieeg_epilepsyNWB") + source_dpath = bids_examples / "ieeg_epilepsyNWB" + dpath = tmp_path / "ieeg_epilepsyNWB" shutil.copytree(source_dpath, dpath) - file_path = os.path.join( - dpath, - *"sub-01/ses-postimp/ieeg/sub-01_ses-postimp_task-seizure_run-01_ieeg.nwb".split( - "/" - ) + file_path = ( + dpath + / "sub-01" + / "ses-postimp" + / "ieeg" + / "sub-01_ses-postimp_task-seizure_run-01_ieeg.nwb" ) metadata = get_metadata(file_path) # This is a key sourced from both NWB and BIDS: @@ -726,7 +726,7 @@ def test_ndtypes(ndtypes, asset_dict): @mark.skipif_no_network -def test_nwb2asset(simple2_nwb: str) -> None: +def test_nwb2asset(simple2_nwb: Path) -> None: assert nwb2asset(simple2_nwb, digest=DUMMY_DANDI_ETAG) == BareAsset.unvalidated( schemaKey="Asset", schemaVersion=DANDI_SCHEMA_VERSION, @@ -767,7 +767,7 @@ def test_nwb2asset(simple2_nwb: str) -> None: contentSize=19664, encodingFormat="application/x-nwb", digest={DigestType.dandi_etag: "dddddddddddddddddddddddddddddddd-1"}, - path=simple2_nwb, + path=str(simple2_nwb), dateModified=ANY_AWARE_DATETIME, blobDateModified=ANY_AWARE_DATETIME, wasAttributedTo=[ diff --git a/dandi/tests/test_organize.py b/dandi/tests/test_organize.py index 7eb203470..990157e99 100644 --- a/dandi/tests/test_organize.py +++ b/dandi/tests/test_organize.py @@ -1,6 +1,5 @@ from __future__ import annotations -from glob import glob import os import os.path as op from pathlib import Path @@ -111,8 +110,8 @@ def c() -> Any: # shortcut @pytest.mark.integration @pytest.mark.parametrize("mode", no_move_modes) -def test_organize_nwb_test_data(nwb_test_data: str, tmp_path: Path, mode: str) -> None: - outdir = str(tmp_path / "organized") +def test_organize_nwb_test_data(nwb_test_data: Path, tmp_path: Path, mode: str) -> None: + outdir = tmp_path / "organized" relative = False if mode == "symlink-relative": @@ -123,8 +122,8 @@ def test_organize_nwb_test_data(nwb_test_data: str, tmp_path: Path, mode: str) - # organize also organize using relative paths in case of 'symlink' # mode cwd = os.getcwd() - nwb_test_data = op.relpath(nwb_test_data, cwd) - outdir = op.relpath(outdir, cwd) + nwb_test_data = Path(op.relpath(nwb_test_data, cwd)) + outdir = Path(op.relpath(outdir, cwd)) src = tmp_path / "src" src.touch() @@ -151,16 +150,16 @@ def test_organize_nwb_test_data(nwb_test_data: str, tmp_path: Path, mode: str) - elif mode == "hardlink" and not hardlinks_work: pytest.skip("Hard links not supported") - input_files = op.join(nwb_test_data, "v2.0.1") + input_files = nwb_test_data / "v2.0.1" - cmd = ["-d", outdir, "--files-mode", mode, input_files] + cmd = ["-d", str(outdir), "--files-mode", mode, str(input_files)] r = CliRunner().invoke(organize, cmd) # with @map_to_click_exceptions we loose original str of message somehow # although it is shown to the user - checked. TODO - figure it out # assert "not containing all" in str(r.exc_info[1]) assert r.exit_code != 0, "Must have aborted since many files lack subject_id" - assert not glob(op.join(outdir, "*")), "no files should have been populated" + assert not any(outdir.glob("*")), "no files should have been populated" r = CliRunner().invoke(organize, cmd + ["--invalid", "warn"]) assert r.exit_code == 0 @@ -187,19 +186,19 @@ def test_organize_nwb_test_data(nwb_test_data: str, tmp_path: Path, mode: str) - assert not any(op.islink(p) for p in produced_paths) -def test_ambiguous(simple2_nwb: str, tmp_path: Path) -> None: +def test_ambiguous(simple2_nwb: Path, tmp_path: Path) -> None: copy2 = copy_nwb_file(simple2_nwb, tmp_path) - outdir = str(tmp_path / "organized") - args = ["--files-mode", "copy", "-d", outdir, simple2_nwb, copy2] + outdir = tmp_path / "organized" + args = ["--files-mode", "copy", "-d", str(outdir), str(simple2_nwb), copy2] r = CliRunner().invoke(organize, args) assert r.exit_code == 0 - produced_paths = sorted(find_files(".*", paths=outdir)) - produced_paths_rel = [op.relpath(p, outdir) for p in produced_paths] - assert produced_paths_rel == sorted( - op.join( - "sub-mouse001", "sub-mouse001_obj-%s.nwb" % get_obj_id(get_object_id(f)) - ) - for f in [simple2_nwb, copy2] + assert list_paths(outdir) == sorted( + [ + outdir + / "sub-mouse001" + / f"sub-mouse001_obj-{get_obj_id(get_object_id(f))}.nwb" + for f in [simple2_nwb, Path(copy2)] + ] ) @@ -350,7 +349,7 @@ def test_validate_organized_path(path: str, error_ids: list[str]) -> None: assert [e.id for e in errors] == error_ids -def test_organize_required_field(simple2_nwb: str, tmp_path: Path) -> None: +def test_organize_required_field(simple2_nwb: Path, tmp_path: Path) -> None: (tmp_path / dandiset_metadata_file).write_text("{}\n") r = CliRunner().invoke( organize, @@ -360,7 +359,7 @@ def test_organize_required_field(simple2_nwb: str, tmp_path: Path) -> None: "--dandiset-path", str(tmp_path), "--required-field=session_id", - simple2_nwb, + str(simple2_nwb), ], ) assert r.exit_code == 0 diff --git a/dandi/tests/test_pynwb_utils.py b/dandi/tests/test_pynwb_utils.py index 298d973ff..9f19998bd 100644 --- a/dandi/tests/test_pynwb_utils.py +++ b/dandi/tests/test_pynwb_utils.py @@ -1,4 +1,5 @@ from datetime import datetime, timezone +from pathlib import Path import re from typing import Any, Callable, NoReturn @@ -8,10 +9,10 @@ from ..pynwb_utils import _sanitize_nwb_version, nwb_has_external_links -def test_pynwb_io(simple1_nwb: str) -> None: +def test_pynwb_io(simple1_nwb: Path) -> None: # To verify that our dependencies spec is sufficient to avoid # stepping into known pynwb/hdmf issues - with NWBHDF5IO(str(simple1_nwb), "r", load_namespaces=True) as reader: + with NWBHDF5IO(simple1_nwb, "r", load_namespaces=True) as reader: nwbfile = reader.read() assert repr(nwbfile) assert str(nwbfile) diff --git a/dandi/tests/test_validate.py b/dandi/tests/test_validate.py index e80552e2b..90e51cd2a 100644 --- a/dandi/tests/test_validate.py +++ b/dandi/tests/test_validate.py @@ -1,6 +1,5 @@ import json -import os -import pathlib +from pathlib import Path import pytest @@ -8,25 +7,21 @@ from ..validate import validate -def test_validate_nwb_error(simple3_nwb): - """ - Do we fail on critical NWB validation errors? - """ - +def test_validate_nwb_error(simple3_nwb: Path) -> None: + """Do we fail on critical NWB validation errors?""" validation_result = validate(simple3_nwb) assert len([i for i in validation_result if i.severity]) > 0 @pytest.mark.parametrize("dataset", BIDS_TESTDATA_SELECTION) -def test_validate_bids(bids_examples, tmp_path, dataset): - - selected_dataset = os.path.join(bids_examples, dataset) +def test_validate_bids(bids_examples: Path, tmp_path: Path, dataset: str) -> None: + selected_dataset = bids_examples / dataset validation_result = validate(selected_dataset) for i in validation_result: assert i.severity is None -def test_validate_bids_onefile(bids_error_examples, tmp_path): +def test_validate_bids_onefile(bids_error_examples: Path, tmp_path: Path) -> None: """ Dedicated test using single-file validation. @@ -39,37 +34,30 @@ def test_validate_bids_onefile(bids_error_examples, tmp_path): a whole anyway. """ - selected_dataset, error_file = ( - "invalid_asl003", - "sub-Sub1/perf/sub-Sub1_headshape.jpg", - ) + selected_dataset = "invalid_asl003" + error_file = Path("sub-Sub1/perf/sub-Sub1_headshape.jpg") - bids_file_path = os.path.join(bids_error_examples, selected_dataset, error_file) - bids_dataset_path = pathlib.Path( - bids_error_examples, selected_dataset - ) - error_reference = os.path.join(bids_dataset_path, ".ERRORS.json") - with open(error_reference) as f: + bids_file_path = bids_error_examples / selected_dataset / error_file + error_reference = bids_error_examples / selected_dataset / ".ERRORS.json" + with error_reference.open() as f: expected_errors = json.load(f) validation_result = validate(bids_file_path) for i in validation_result: error_id = i.id - error_path = i.path - relative_error_path = os.path.relpath(error_path, i.dataset_path) - relative_error_path = pathlib.Path(relative_error_path).as_posix() + assert i.path is not None + assert i.dataset_path is not None + relative_error_path = i.path.relative_to(i.dataset_path).as_posix() assert relative_error_path in expected_errors[error_id.lstrip("BIDS.")]["scope"] @pytest.mark.parametrize("dataset", BIDS_ERROR_TESTDATA_SELECTION) -def test_validate_bids_errors(bids_error_examples, dataset): - # This only checks that the error we found is correct, not that we found all errors. - # ideally make a list and erode etc. - - selected_dataset = os.path.join(bids_error_examples, dataset) - validation_result = validate(selected_dataset) - with open(os.path.join(selected_dataset, ".ERRORS.json")) as f: +def test_validate_bids_errors(bids_error_examples: Path, dataset: str) -> None: + # This only checks that the error we found is correct, not that we found + # all errors. ideally make a list and erode etc. + selected_dataset = bids_error_examples / dataset + validation_result = list(validate(selected_dataset)) + with (selected_dataset / ".ERRORS.json").open() as f: expected_errors = json.load(f) - validation_result = [i for i in validation_result] # We know that these datasets contain errors. assert len(validation_result) > 0 @@ -79,10 +67,9 @@ def test_validate_bids_errors(bids_error_examples, dataset): if i.id == "BIDS.MATCH": continue error_id = i.id - if i.path: - error_path = i.path - relative_error_path = os.path.relpath(error_path, i.dataset_path) - relative_error_path = pathlib.Path(relative_error_path).as_posix() + if i.path is not None: + assert i.dataset_path is not None + relative_error_path = i.path.relative_to(i.dataset_path).as_posix() assert ( relative_error_path in expected_errors[error_id.lstrip("BIDS.")]["scope"] diff --git a/dandi/validate.py b/dandi/validate.py index 0bdfec666..509dd0bc3 100644 --- a/dandi/validate.py +++ b/dandi/validate.py @@ -130,7 +130,7 @@ def validate_bids( def validate( - *paths: str, + *paths: str | Path, schema_version: Optional[str] = None, devel_debug: bool = False, allow_any_path: bool = False,