[New Check]: Non-required Electrode columns (nwb-schema >= 2.5.0)#225
Draft
CodyCBakerPhD wants to merge 11 commits intodevfrom
Draft
[New Check]: Non-required Electrode columns (nwb-schema >= 2.5.0)#225CodyCBakerPhD wants to merge 11 commits intodevfrom
CodyCBakerPhD wants to merge 11 commits intodevfrom
Conversation
CodyCBakerPhD
added
the
category: new check
bendichter
reviewed
Jul 11, 2022
| if get_package_version(name="pynwb") < version.Version("2.1.0"): | ||
| return | ||
| if any(x.isnan() in electrode_table for x in ["x", "y", "z", "imp", "filtering"]): | ||
| return InspectorMessage( |
Contributor
There was a problem hiding this comment.
I would yield here and say in the message which column is Nan. Also, does isnan work for an array? Maybe we could say if np.all(np.isnan(col))
Contributor
Author
There was a problem hiding this comment.
Sure, do you want it to report the exact index(ices) that are NaN or just the column that it was found in?
bendichter
reviewed
Jul 11, 2022
| group=ElectrodeGroup(name="test_group", description="", device=Device(name="test_device"), location="unknown"), | ||
| group_name="test_group", | ||
| ) | ||
| if get_package_version(name="pynwb") >= version.Version("2.1.0"): |
Contributor
There was a problem hiding this comment.
I think this logic should go inside the skip statement. From the comments it seems like maybe you agree?
Contributor
Author
There was a problem hiding this comment.
This one is temporary until they fix the bug - this PR will have to stay in draft until then.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix #220
Early work on adding this check. Currently relies (incorrectly) on grabbing this information from the PyNWB version being used to read the files (>= 2.1.0 release) however that would not correctly apply to a file written using a cached namespace corresponding to any
nwb-schema<2.5.0.A couple of design considerations...
i) how to get the cached namespace core version? I tried the
NWBHDF5IOnwbfile and such for this but didn't have any luck -nwbfile.namespacesjust reports the string"core"and not the version. I was able to get this information however by the followingapplied to a
file = h5py.File(...)ii) how best to communicate this version information from the io/file reading level of the
inspect_nwb/run_checkslevel of the inspector and the ultimate check function (which is really only designed to receive one input, that being the object being checked.