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

Support NWB schema 2.2.2 #1146

Merged
merged 56 commits into from
Mar 4, 2020
Merged

Support NWB schema 2.2.2 #1146

merged 56 commits into from
Mar 4, 2020

Conversation

rly
Copy link
Contributor

@rly rly commented Jan 17, 2020

This is a work in progress to support the changes in the NWB schema between versions 2.1.0 and 2.2.2.

  • Moved common data structures such as Container and DynamicTable to hdmf-common-schema. The hdmf-common-schema repo is now included as a submodule. See Remove HDMF common data structures nwb-schema#307 for details

    • Make sure imports are still available from original location
  • Added "channel_conversion" dataset to ElectricalSeries to represent per-channel conversion factors.

  • Added "sampling_rate" and "unit" attributes to "waveform_mean" and "waveform_sd" datasets/columns in Units table.

  • Added "description" and "manufacturer" attributes to Device.

  • Deprecated ImagingPlane "manifold" in favor of "origin_coords" and "grid_spacing"

  • Use "text" data type for all DynamicTable "colnames". Previously, only ASCII was allowed.

  • Use "text" data type for electrode table columns "location" and "group_name". Previously, only ASCII was allowed.

  • Added to description to make electrode x,y,z consistent with CCF reference. http://help.brain-map.org/display/mousebrain/API#API-DownloadAtlas3-DReferenceModels

  • Added "position" dataset with compound data type x,y,z in ElectrodeGroup.

  • Avoid enforcing "uint64" for sweep numbers for better compatibility. Use uint instead which is 32bit.

  • Set dtype for Image and its subtypes to numeric. (note: technically this breaks backwards compatibility, in the schema, but the pynwb API has always enforced that Images have a numeric type, and realistically we do not think users are storing strings in an Image dataset.)

  • Added "resolution" attribute to "spike_times" column of Units.

  • Removed "required" key from dataset ImageSeries.field_of_view for schema language compliance.

  • Replaced "required" keys with "quantity" keys for ImagingPlane.origin_coords and ImagingPlane.grid_spacing for schema language compliance.

  • Refactored ImagingRetinotopy type to reduce redundancy.

  • Added "doc" key to ImagingRetinotopy.axis_2_power_map for schema language compliance.

  • Fixed makefiles for generating documentation on Windows.

  • Added optional "reference" column in "electrodes" table.

  • Changed dims of ImageSeries from (frame, y, x) to (frame, x, y) and (frame, z, y, x) to (frame, x, y, z) to be consistent with the dimension ordering in PlaneSegmentation.

  • Changed dims of Image from (y, x) to (x, y). (note: as far as we know, users of NWB 2.0 that use the - - [ ] Image type encode their data as (x, y)) to be consistent with the dimension ordering in ImageSeries.

  • Updated hdmf-common-schema to version 1.1.0 which includes:
    The 'colnames' attribute of DynamicTable changed from data type 'ascii' to 'text'.
    Improved documentation and type docstrings.

  • Fix shape and dims of OpticalSeries.data for color images

  • Allow more than one OpticalChannel object in ImagingPlane

  • Update hdmf-common-schema to 1.1.3. This fixes missing 'shape' and 'dims' key for types VectorData, VectorIndex, and DynamicTableRegion.

  • Revert changes to retinotopy.yaml in 2.1.0 which break backward compatibility and were not supported by the APIs in any case. Changes will be revisited in a future version.

rly and others added 4 commits January 17, 2020 14:14
* adjust ImagingPlane arg order to maintain backwards compatibility
tests:
* cast sweep_number as uint
* fix position assertion
@codecov
Copy link

codecov bot commented Jan 20, 2020

Codecov Report

❗ No coverage uploaded for pull request base (dev@9ef4128). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##             dev    #1146   +/-   ##
======================================
  Coverage       ?   67.97%           
======================================
  Files          ?       37           
  Lines          ?     2398           
  Branches       ?      423           
======================================
  Hits           ?     1630           
  Misses         ?      697           
  Partials       ?       71
Impacted Files Coverage Δ
src/pynwb/__init__.py 72.26% <ø> (ø)
src/pynwb/file.py 73.74% <100%> (ø)
src/pynwb/io/file.py 78.41% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9ef4128...7068c53. Read the comment docs.

@bendichter
Copy link
Contributor

Adding "reference" to electrodes has been tricky

@wbwakeman
Copy link

Hi @rly . I'm trying to plan our NWB upgrade path for AllenSDK. Can you provide an estimate on when a PyNWB release will be available that targets NWB schema 2.2.1?

@rly
Copy link
Contributor Author

rly commented Feb 28, 2020

@wbwakeman I am doing final testing of this PR and the corresponding release of HDMF. The targeted release is end of day Monday (March 2).

@rly rly marked this pull request as ready for review March 3, 2020 03:12
@rly rly changed the title Support NWB schema 2.2.1 Support NWB schema 2.2.2 Mar 3, 2020
@rly
Copy link
Contributor Author

rly commented Mar 3, 2020

@oruebel @ajtritt This PR is ready to go after tests pass. Please review. After this is merged and any other ready-to-go PRs are merged, I will release PyNWB 1.3.0.

@rly
Copy link
Contributor Author

rly commented Mar 3, 2020

This is a large PR encompassing several changes to the API in order to support NWB schema 2.2.1 and 2.2.2. In the future, we will have a staging branch for support of the next release of nwb-schema. Individual PRs in PyNWB corresponding to new changes in nwb-schema will be squashed and merged into the staging branch. And when a new version of nwb-schema is released, a PR will be made in PyNWB for merging the staging branch into dev. This merge will not be squashed in order to keep the individual, original PRs.

@rly
Copy link
Contributor Author

rly commented Mar 4, 2020

Thanks @oruebel and @ajtritt!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants