Update to nwb schema 2.2.1

[NileGraddis]

This is a dev branch for our update to a newer pynwb. New work should target this branch.

[t-b]

I'm pretty sure that the error in test_valid_v2_full_ABF[2018_03_20_0005.nwb] - tests.test_nwb_reader is due to the new hardcoded units for patchclampseries.

[sgratiy]

@wbwakeman This branch should be ready to run ephys pipeline on the nwb 2 files. This branch included tests that check that pipeline output between nwb1 and nwb2 are identical. We will merge this branch into master when pynwb merges NeurodataWithoutBorders/pynwb#1146

[t-b]

@sgratiy @NileGraddis In case I should review this, could you rebase?

[sgratiy]

@t-b @NileGraddis I have rebased it onto master

[t-b]

Review:

811a22d (update pynwb req, 2020-02-13)
f1b4e9f (unpin hdmf, 2020-02-13)

Good. A bit on the short side of commit message though.

21e4ffe (ipfx/nwb_reader.py: Accept more diverse units in NwbReader.get_long_unit_name, 2020-02-01)
f42b725 (ipfx/nwb_reader.py: Remove unused code in NwbXReader.get_sweep_data(), 2020-02-01)
fbde93b (ipfx/nwb_reader.py: Adapt get_nwb_version() for newer NWB v2 files, 2020-02-01)
4419866 (Tests: Add testcase with MIES NWBv2 data, 2020-02-01)

My commits.

77937c0 (Add 'amperes' as a valid SI unit string, 2020-02-14)

9451434 (skip tests for the nwb converter that are temporary failing due to pynwb changes, 2020-02-14)

It would have been nice to link to the upstream issue/PR, NeurodataWithoutBorders/pynwb#1188 is the correct one.

In the meantime this is now also fixed which I tried to address in b04b1dc.

Edit: I don't know why it fails on circleci, it passes here.

cc7f9c2 (Add MIES nwb2 test, 2020-02-14)

I can't test that here but it should not contain print statements or commented out code. flake8 is also reporting E127 continuation line over-indented for visual indent.

af01f7a (Remote trailing _DA_0 from stimulus code to be consistent with nwb1 reader, 2020-02-14)

Typo Remote -> Remove

d18eb8e (Add MiesDataSet class to handle Mies nwb2 that include labnotebook, 2020-02-14)

def is_file_mies(path: str) -> bool:

Are we okay with requiring python 3.5 as this type hint suggests? I'm just asking, it should be a concise choice.

Rewriting create_data_set to something like

    if nwb_version["major"] == 2 and is_file_mies(nwb_file):

would be faster for NWBv1 files as don't need to open it in this case, but this is purely optional.

No love for local functions here? Well fine with me ;)

31f1929 (set scalar attributes in nwb file, 2020-02-14)

Functionality wise okay, but I would love to see longer commit messages explaining why the change was needed.

2526645 (Fix test: remove trailing _DA_0 from stim code, 2020-02-14)

That should probably be squashed into the commit breaking the test.

22f7076 (MiesDataSet object, 2020-02-14)

Looks good. One thing which is a general issue:

        cnt = self.notebook.get_value("Set Sweep Count", sweep_num, 0)

I don't like the default of 0 here. In MIES we have since 89c34402 (Sweep settings stored, 2014-09-07) stored the set sweep count so I can not imagine a case where this is not present.
This is also a general issue:

$ git grep "Set Sweep Count" .
aibs_data_set.py:        cnt = self.notebook.get_value("Set Sweep Count", sweep_num, 0)
mies_data_set.py:        cnt = self.notebook.get_value("Set Sweep Count", sweep_num, 0)
x_to_nwb/NWBConverter.py:        cnt = self.notebook.get_value("Set Sweep Count", sweep_num, 0)

6ee232c (#321 tests for mies data set, 2020-02-17)

This uses an f string so this is python 3.6, just mentioning.

ddf31b1 (#321 update minimum h5py (some mies datasets cannot be read prior to 2.10), 2020-02-17)

Jeep.

563a9a6 (#321 ensure that aibs fs-requiring tests are skipped if the aibs fs is not available, 2020-02-17)
daf9bbf (Add nwb tests to cover all versions of patchseq, 2020-02-18)
5f9fee7 (Explicitly specify valid SI unit, 2020-02-19)

All Good.

[t-b]

See comment.

[sgratiy]

@t-b Thanks for reviewing, and helpful suggestions. We are basing our new work off of this branch until pynwb released support for the schema 2.2.1. Thus I would have to revert your commit, to avoid dealing with rewriting history of our new work. Could you please instead make a PR against thins branch? Thanks.

[t-b]

@sgratiy Sure! See #337.

[t-b]

@sgratiy Is there any change in plans in adopting newer schema versions?

[sgratiy]

No there is no change. This branch was already merged in to the dev branch where we are doing the development.