read error for tutorial/ecephys

[bendichter]

tutorial/ecephys write correctly but has the following error on read. I think this may be a bug, since it writes but does not read.

Error using types.util.checkDtype (line 92)
Property `description` must be a char.

Error in
types.core.DynamicTable/validate_description (line
59)
        val = types.util.checkDtype('description',
        'char', val);

Error in types.core.DynamicTable/set.description
(line 45)
        obj.description =
        obj.validate_description(val);

Error in types.core.DynamicTable (line 33)
        obj.description = p.Results.description;

Error in io.parseGroup (line 71)
    parsed = eval([typename '(kwargs{:})']);

Error in io.parseGroup (line 28)
    subg = io.parseGroup(filename, g_info);

Error in io.parseGroup (line 28)
    subg = io.parseGroup(filename, g_info);

Error in io.parseGroup (line 28)
    subg = io.parseGroup(filename, g_info);

Error in nwbRead (line 20)
    nwb = io.parseGroup(filename, info);

Error in ecephys (line 228)
nwb2 = nwbRead('ecephys_tutorial.nwb');

[lawrence-mbf]

This looks like a naming conflict. With /general/extracellular_ephys/electrodes being a DynamicTable, it now has two properties named description, one expecting a string, and the another expecting a column. Since Attributes, Datasets, and Subgroups share the same namespace on read, but not on write due to how constrained groups are stored in the structure.

[lawrence-mbf]

Yeah scratch that, it might be best to change the name in this case. Fixing this on the backend will require rewriting inputParser.

[bendichter]

Ok.. I think this will require a schema change though

On Wed, Oct 10, 2018, 8:22 AM ln-vidrio notifications@github.com wrote:

Yeah scratch that, it might be best to change the name in this case. Fixing this on the backend will require rewriting inputParser.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/NeurodataWithoutBorders/matnwb/issues/72#issuecomment-428615569, or mute the thread https://github.com/notifications/unsubscribe-auth/AAziEv5ylgxrYQpFNPaM9Kb3NoB4zB5Jks5ujhC1gaJpZM4XUS8r .

[lawrence-mbf]

Oh you're right, didn't notice that. I'll think of a solution on this end then.

[bendichter]

It probably wouldn't be that big a deal to change the description attribute to table_description if we need to.

On Wed, Oct 10, 2018 at 8:46 AM ln-vidrio notifications@github.com wrote:

Oh you're right, didn't notice that. I'll think of a solution on this end then.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/NeurodataWithoutBorders/matnwb/issues/72#issuecomment-428624615, or mute the thread https://github.com/notifications/unsubscribe-auth/AAziEnQKRStatoqErFo-Z9mX52JxFZPFks5ujhY4gaJpZM4XUS8r .

--

Ben Dichter, PhD Data Science Consultant

[bendichter]

If we do make this change, we should still create a check on write to make sure that we don't have a naming collision, in order to avoid this read error.

[lawrence-mbf]

The main reason I have against changing the schema is that what the schema's doing is not breaking. I'm working on it right now and I think I have something that works (albeit, only for constrained elements).

[bendichter]

wait, I was wrong, description was removed in the latest update

[bendichter]

I fixed it just by removing the "description" column and the tutorial now works. Though I think this is still worth addressing somehow to prevent read errors.

[lawrence-mbf]

I'm confused, is description still a column in the core schema?

[bendichter]

no, it was recently removed. However in a dynamic table it can be added (which is what I was inadvertently doing), and that will cause read errors.

[lawrence-mbf]

3cb078a587c5e2efaa91b50b6f4a16f3fe7dbd61 This should now work even with the duplicated 'description' problem.