From 788a888da8afbdf46768ddc13099910490806c0b Mon Sep 17 00:00:00 2001 From: Cody Baker Date: Tue, 20 Sep 2022 09:28:23 -0400 Subject: [PATCH 1/6] fix cell_id error --- nwbinspector/checks/icephys.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nwbinspector/checks/icephys.py b/nwbinspector/checks/icephys.py index 0358e6530..841cdc796 100644 --- a/nwbinspector/checks/icephys.py +++ b/nwbinspector/checks/icephys.py @@ -6,5 +6,5 @@ @register_check(importance=Importance.BEST_PRACTICE_VIOLATION, neurodata_type=IntracellularElectrode) def check_intracellular_electrode_cell_id_exists(intracellular_electrode: IntracellularElectrode): """Check if the IntracellularElectrode contains a cell_id.""" - if intracellular_electrode.cell_id is None: + if getattr(intracellular_electrode, "cell_id", None) is None: return InspectorMessage(message="Please include a unique cell_id associated with this IntracellularElectrode.") From b3016a0a00eb9a323dde1df8c4a70e8de8cc9df4 Mon Sep 17 00:00:00 2001 From: Cody Baker Date: Tue, 20 Sep 2022 09:32:19 -0400 Subject: [PATCH 2/6] added tests as well --- .github/workflows/version_gallery.yml | 32 +++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 .github/workflows/version_gallery.yml diff --git a/.github/workflows/version_gallery.yml b/.github/workflows/version_gallery.yml new file mode 100644 index 000000000..44872daa2 --- /dev/null +++ b/.github/workflows/version_gallery.yml @@ -0,0 +1,32 @@ +name: Past PyNWB Version +on: + schedule: + - cron: "0 0 * * *" # daily + pull_request: + +jobs: + build-and-test: + name: Testing against past PyNWB versions + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + pynwb-version: ["2.1.0", "2.0.0"] + steps: + - uses: s-weigand/setup-conda@v1 + with: + update-conda: true + python-version: 3.9 + conda-channels: conda-forge + - uses: actions/checkout@v2 + - run: git fetch --prune --unshallow --tags + - name: Install pytest + run: | + pip install pytest + pip install pytest-cov + - name: Install package + run: | + pip install -e . + pip install pynwb==${{ matrix.pynwb-version }} + - name: Run pytest with coverage + run: pytest -rsx From 1231dcaab0ab969cffcd0425a78afab6033de03c Mon Sep 17 00:00:00 2001 From: Cody Baker Date: Tue, 20 Sep 2022 09:51:01 -0400 Subject: [PATCH 3/6] include test skips --- tests/unit_tests/test_icephys.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/unit_tests/test_icephys.py b/tests/unit_tests/test_icephys.py index 59e4ee6fa..511f2667d 100644 --- a/tests/unit_tests/test_icephys.py +++ b/tests/unit_tests/test_icephys.py @@ -1,15 +1,23 @@ +import pytest +from packaging.version import Version from pynwb.icephys import IntracellularElectrode from pynwb.device import Device from nwbinspector import InspectorMessage, Importance, check_intracellular_electrode_cell_id_exists +from nwbinspector.utils import get_package_version +PYNWB_VERSION_SKIP = get_package_version(name="pynwb") < Version("2.1.0") +PYNWB_VERSION_SKIP_REASON = "This test requires PyNWB>=2.1.0" + +@pytest.mark.skipif(PYNWB_VERSION_SKIP, reason=PYNWB_VERSION_SKIP_REASON) def test_pass_check_intracellular_electrode_cell_id_exists(): device = Device(name="device") ielec = IntracellularElectrode(name="ielec", cell_id="123", device=device, description="an intracellular electrode") assert check_intracellular_electrode_cell_id_exists(ielec) is None +@pytest.mark.skipif(PYNWB_VERSION_SKIP, reason=PYNWB_VERSION_SKIP_REASON) def test_fail_check_intracellular_electrode_cell_id_exists(): device = Device(name="device") ielec = IntracellularElectrode(name="ielec", device=device, description="an intracellular electrode") From f158d33e083d78272b2eb9101ab0d4c9929f9936 Mon Sep 17 00:00:00 2001 From: Cody Baker Date: Tue, 20 Sep 2022 09:53:36 -0400 Subject: [PATCH 4/6] add alternative test --- tests/unit_tests/test_icephys.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/tests/unit_tests/test_icephys.py b/tests/unit_tests/test_icephys.py index 511f2667d..fe3fa02a7 100644 --- a/tests/unit_tests/test_icephys.py +++ b/tests/unit_tests/test_icephys.py @@ -6,18 +6,18 @@ from nwbinspector import InspectorMessage, Importance, check_intracellular_electrode_cell_id_exists from nwbinspector.utils import get_package_version -PYNWB_VERSION_SKIP = get_package_version(name="pynwb") < Version("2.1.0") -PYNWB_VERSION_SKIP_REASON = "This test requires PyNWB>=2.1.0" +PYNWB_VERSION_LOWER_2_1_0 = get_package_version(name="pynwb") < Version("2.1.0") +PYNWB_VERSION_LOWER_SKIP_REASON = "This test requires PyNWB>=2.1.0" -@pytest.mark.skipif(PYNWB_VERSION_SKIP, reason=PYNWB_VERSION_SKIP_REASON) +@pytest.mark.skipif(PYNWB_VERSION_LOWER_2_1_0, reason=PYNWB_VERSION_LOWER_SKIP_REASON) def test_pass_check_intracellular_electrode_cell_id_exists(): device = Device(name="device") ielec = IntracellularElectrode(name="ielec", cell_id="123", device=device, description="an intracellular electrode") assert check_intracellular_electrode_cell_id_exists(ielec) is None -@pytest.mark.skipif(PYNWB_VERSION_SKIP, reason=PYNWB_VERSION_SKIP_REASON) +@pytest.mark.skipif(PYNWB_VERSION_LOWER_2_1_0, reason=PYNWB_VERSION_LOWER_SKIP_REASON) def test_fail_check_intracellular_electrode_cell_id_exists(): device = Device(name="device") ielec = IntracellularElectrode(name="ielec", device=device, description="an intracellular electrode") @@ -29,3 +29,10 @@ def test_fail_check_intracellular_electrode_cell_id_exists(): object_name="ielec", location="/", ) + + +@pytest.mark.skipif(not PYNWB_VERSION_LOWER_2_1_0, reason="This test requires PyNWB<2.1.0") +def test_skip_check_for_lower_versions(): + device = Device(name="device") + ielec = IntracellularElectrode(name="ielec", device=device, description="an intracellular electrode") + assert check_intracellular_electrode_cell_id_exists(ielec) is None From 071f167bf9686ef65f94375b2494c35fd81fd558 Mon Sep 17 00:00:00 2001 From: Cody Baker Date: Tue, 20 Sep 2022 10:02:26 -0400 Subject: [PATCH 5/6] try swapping to hasattr --- nwbinspector/checks/icephys.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nwbinspector/checks/icephys.py b/nwbinspector/checks/icephys.py index 841cdc796..860235d9e 100644 --- a/nwbinspector/checks/icephys.py +++ b/nwbinspector/checks/icephys.py @@ -6,5 +6,5 @@ @register_check(importance=Importance.BEST_PRACTICE_VIOLATION, neurodata_type=IntracellularElectrode) def check_intracellular_electrode_cell_id_exists(intracellular_electrode: IntracellularElectrode): """Check if the IntracellularElectrode contains a cell_id.""" - if getattr(intracellular_electrode, "cell_id", None) is None: + if getattr(intracellular_electrode, "cell_id", "") is None: # Will only be None with PyNWB>=2.1.0 return InspectorMessage(message="Please include a unique cell_id associated with this IntracellularElectrode.") From a1ee11a8f6daf4f3386de5b15f5bdec1e32a58f0 Mon Sep 17 00:00:00 2001 From: Cody Baker Date: Tue, 20 Sep 2022 14:11:06 -0400 Subject: [PATCH 6/6] actually swap to hasattr --- nwbinspector/checks/icephys.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nwbinspector/checks/icephys.py b/nwbinspector/checks/icephys.py index 860235d9e..c27fdc16f 100644 --- a/nwbinspector/checks/icephys.py +++ b/nwbinspector/checks/icephys.py @@ -6,5 +6,5 @@ @register_check(importance=Importance.BEST_PRACTICE_VIOLATION, neurodata_type=IntracellularElectrode) def check_intracellular_electrode_cell_id_exists(intracellular_electrode: IntracellularElectrode): """Check if the IntracellularElectrode contains a cell_id.""" - if getattr(intracellular_electrode, "cell_id", "") is None: # Will only be None with PyNWB>=2.1.0 + if hasattr(intracellular_electrode, "cell_id") and intracellular_electrode.cell_id is None: return InspectorMessage(message="Please include a unique cell_id associated with this IntracellularElectrode.")