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

Electrodes table schema change #176

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Electrodes table schema change #176

wants to merge 2 commits into from

Conversation

bendichter
Copy link
Contributor

I am trying to make sure the changes in NeurodataWithoutBorders/nwb-schema#337 don't cause any problems for matnwb. This requires some changes to ecephys, because the electrodes table needs to be defined as an Electrodes table rather than a DynamicTable. When I run ecephys now, I get the following error that I have not been able to track down.

Error using hdf5lib2
The HDF5 library encountered an error and produced the
following stack trace information:

    H5G_loc_find_cb      object 'electrodes' doesn't exist
    H5G_traverse_real    traversal operator failed
    H5G_traverse         internal path traversal failed
    H5G_loc_find         can't find object
    H5O_open_name        object not found
    H5Oopen              unable to open object

Error in H5O.open (line 27)
output = H5ML.hdf5lib2('H5Oopen',loc_id,relname,lapl_id);

Error in io.writeAttribute (line 8)
oid = H5O.open(fid, path, 'H5P_DEFAULT');

Error in types.untyped.MetaClass/export (line 66)
                io.writeAttribute(fid, namespacePath,
                namespace);

Error in types.hdmf_common.Container/export (line 21)
        refs = [email protected](obj, fid,
        fullpath, refs);

Error in types.hdmf_common.DynamicTable/export (line 81)
        refs = [email protected]_common.Container(obj, fid,
        fullpath, refs);

Error in types.core.Electrodes/export (line 140)
        refs = [email protected]_common.DynamicTable(obj,
        fid, fullpath, refs);

Error in types.core.NWBFile/export (line 565)
            refs =
            obj.general_extracellular_ephys_electrodes.export(fid,
            [fullpath
            '/general/extracellular_ephys/electrodes'],
            refs);

Error in NwbFile/export (line 55)
                refs = [email protected](obj,
                output_file_id, '/', {});

Error in nwbExport (line 35)
    export(nwb(i), filename);

Error in ecephys (line 301)
nwbExport(nwb, 'ecephys_tutorial.nwb')

@ln-vidrio Do you know what this could be? Could you take a look at this?

@lawrence-mbf
Copy link
Collaborator

The reason for this error has to do with ea9c25a. We now have to implement a way to derive whether or not a class is a Group or a Dataset class. I'll push a few changes to get this working.

The original method for deriving H5 type of a class was not maintainable, so we
decided to use the schema instead.  However, the exporting class also needs this
information as well.

Two empty stub groups have been created as basic tags to identify whether or not
a given class is a Group or a Dataset: types.untyped.IsGroup and
types.untyped.IsDataset.
@bendichter bendichter marked this pull request as ready for review December 31, 2019 19:14
@bendichter
Copy link
Contributor Author

bendichter commented Dec 31, 2019

works for me! I realized I moved this to non-draft status accidentally. We should not merge until NeurodataWithoutBorders/nwb-schema#337 is merged and released

@yarikoptic
Copy link

well, NeurodataWithoutBorders/nwb-schema#337 remains a draft, so may be this one should get -re-drafted too or just closed? over 3 years has passed

@rly rly marked this pull request as draft February 24, 2023 21:49
@rly
Copy link
Contributor

rly commented Feb 24, 2023

Thanks for the nudge @yarikoptic .

I converted this back to draft. @lawrence-mbf it's been a while but has this issue been resolved?

The reason for this error has to do with ea9c25a. We now have to implement a way to derive whether or not a class is a Group or a Dataset class. I'll push a few changes to get this working.

It would be nice to finally make this ElectrodeTable change in the schema.

@lawrence-mbf
Copy link
Collaborator

@rly I don't really remember but there is something like what I outlined for detecting Datasets and Groups in the current master so I'll assume so.

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.

4 participants