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

replace deprecated ImagingPlane parameters #65

Merged
merged 11 commits into from
Aug 27, 2020
Merged

Conversation

alessandrofelder
Copy link
Collaborator

@alessandrofelder alessandrofelder commented Aug 18, 2020

The ImagingPlane constructor arguments "manifold", "unit" and "conversion" are deprecated, starting from pynwb 1.3.0.
This commit bumps the pynwb version up to 1.3.2 and replaces these deprecated parameters with recommended replacements.

Addresses #63 .

@ageorgou
Copy link
Contributor

ageorgou commented Aug 19, 2020

I think we should merge this into the main version after we are done with the new-labview changes (which should be soon).

@alessandrofelder alessandrofelder changed the base branch from new-labview to master August 19, 2020 17:08
@alessandrofelder alessandrofelder marked this pull request as ready for review August 19, 2020 17:09
Copy link
Contributor

@ageorgou ageorgou left a comment

Choose a reason for hiding this comment

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

This looks good! We should wait until the new-labview changes are in. I don't think there will be conflicts but we'll have to check again.

Comment on lines 35 to 40
# ('161215_15_58_52'),
# ('161215_15_34_21'),
# ('170317_10_11_01'),
# ('170322_14_06_43'),
# ('161222_16_18_47'),
# ('170714_22_26_27')
Copy link
Contributor

Choose a reason for hiding this comment

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

These should be reenabled before merging!

Copy link
Contributor

@ageorgou ageorgou left a comment

Choose a reason for hiding this comment

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

Just a couple of small changes, and perhaps rebase it against the current master for clarity, otherwise this looks ready.

src/silverlabnwb/nwb_file.py Outdated Show resolved Hide resolved
src/silverlabnwb/nwb_file.py Outdated Show resolved Hide resolved
The ImagingPlane constructor arguments "manifold", "unit" and
"conversion" are deprecated from pynwb 1.3.0.
This commit bumps the pynwb version up to 1.3.2 and replaces this
deprecated parameters with recommended replacements.
grid_spacing written as float64 on Travis, but as float32 on local Windows...
changes in signature due to
 - replacing manifold with origin_coords and grid_spacing
 - having added pixel_time_offsets a few commits ago
improve add_imaging_plane docstring (fix parameter name in docstring)
grid_spacing now consistent with earlier versions of code (implied by
the manifold values)
Some environments would write this as 64 bits, making signature
comparisons harder (and this could not be fixed by casting the
value before the signature, as the value itself would have changed,
not just the number of bits used).
This changes the spacing of the grid slightly compared to when we
were storing the manifold explicitly, but I think the differences
are at atomic scale...
Also updating the signatures for the larger files.
@alessandrofelder alessandrofelder merged commit 3c5816c into master Aug 27, 2020
@alessandrofelder alessandrofelder deleted the replace_manifold branch August 27, 2020 16:03
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.

2 participants