Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix attribute retrieval from version-dependent icephys #264

Merged
merged 9 commits into from
Sep 23, 2022
Merged
32 changes: 32 additions & 0 deletions .github/workflows/version_gallery.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
name: Past PyNWB Version
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason you are setting this up as a cron job instead of just adding it to the PR triggered workflow?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiplicity - If I include it in the main workflow we'd have (number of platforms) x (number of Python versions) x (number of PyNWB versions), which gets a bit out of hand.

This instead constrains to ubuntu-latest + Python 3.9 and then goes back as far in the version history as desired.

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
2 changes: 1 addition & 1 deletion nwbinspector/checks/icephys.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 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.")
15 changes: 15 additions & 0 deletions tests/unit_tests/test_icephys.py
Original file line number Diff line number Diff line change
@@ -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_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_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_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")
Expand All @@ -21,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