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.

Sign up for GitHub

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

Jump to bottom

[New Check]: Add check electricalseries of dtype int16 #408

Draft
wants to merge 9 commits into
base: dev
Choose a base branch
from
Draft

[New Check]: Add check electricalseries of dtype int16 #408

wants to merge 9 commits into from

Conversation

alessandratrapani
Copy link
Collaborator

@alessandratrapani alessandratrapani commented Sep 27, 2023 •
edited

Fix #395

Add check on ElectricalSeries data type: if the conversion and offset are DEFAULT values and the data type is any integer ("uint16", "int16", etc).

  • add check function
  • add unit tests
  • add check function documentation in ecephys.rst
  • update CHANGELOG.md

@alessandratrapani alessandratrapani added the category: new check a new best practices check to apply to all NWBFiles and their contents label Sep 27, 2023
@alessandratrapani alessandratrapani self-assigned this Sep 27, 2023
CodyCBakerPhD
CodyCBakerPhD reviewed Sep 27, 2023
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
CodyCBakerPhD and others added 5 commits September 27, 2023 13:10
@CodyCBakerPhD
Merge branch 'dev' into add-check-electricalseries-of-dtype-int16
f513a51
@alessandratrapani @CodyCBakerPhD
change check function
da6afa0 
Co-authored-by: Cody Baker <51133164+CodyCBakerPhD@users.noreply.github.com>
@alessandratrapani @CodyCBakerPhD
explicit float type in test
84e5246 
Co-authored-by: Cody Baker <51133164+CodyCBakerPhD@users.noreply.github.com>
@alessandratrapani @CodyCBakerPhD
Update CHANGELOG.md
f84326e 
Co-authored-by: Cody Baker <51133164+CodyCBakerPhD@users.noreply.github.com>
@alessandratrapani
update test functions
bf3d77e
@CodyCBakerPhD CodyCBakerPhD changed the title Add check electricalseries of dtype int16 [New Check]: Add check electricalseries of dtype int16 Sep 29, 2023
@CodyCBakerPhD
Copy link
Collaborator

Just occurred to me while inspecting some files on the GUIDE; we should also consider the channel_conversion array in this check

If the attribute exists, and the elements of it are not also 1.0, then it's fine to have conversion=1.0 and offset=0.0 because the array of conversion factors is the intended way of specifying that information

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CodyCBakerPhD CodyCBakerPhD CodyCBakerPhD left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@alessandratrapani alessandratrapani

Labels
category: new check a new best practices check to apply to all NWBFiles and their contents
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Add Check]: ElectricalSeries of dtype int16 with conversion=1.0 and offset=0.0 is potentially wrong?
2 participants
@alessandratrapani @CodyCBakerPhD