search

NeurodataWithoutBorders / nwb-guide

NWB GUIDE is a desktop app that provides a no-code user interface for converting neurophysiology data to NWB.
https://nwb-guide.readthedocs.io/
MIT License
21 stars 3 forks source link

Session description is set to empty string by default #887

Open rly opened 1 week ago

rly commented 1 week ago

Describe the issue

NWBFile.session_description is a required dataset in the NWB schema. Going through the tutorial, this field is not set by default. During conversion, the session_description is set to "" (empty string). The NWB inspector does not complain about the session description not being provided. This results in Neurosift showing "Loading..." when reading a file converted by NWB GUIDE: https://github.com/flatironinstitute/neurosift/issues/178

  1. Should the inspector recommend against setting the value to the empty string? I think yes. Just like the other "description" fields.
  2. Should this field be recommended to provide in GUIDE (colored yellow in the form)? I think yes.
  3. Should this field be required to provide in GUIDE (colored red in the form)? I'm not sure. I lean toward yes because it is required in the schema, and empty string is not informative. Alternatively, we decide that this field should not be required in the NWB schema.

How does NeuroConv set session_description?

@CodyCBakerPhD @bendichter

Steps to Reproduce

Convert a file using GUIDE, look at value of session_description

Operating System

Mac OS with Mac M1

GUIDE Version

1.0.1

Code of Conduct

Yes

Did you confirm this issue was not already reported?

Yes

bendichter commented 1 week ago

Yes I would agree having an empty string is not good behavior. If I recall correctly, it might be difficult to make this optional in PyNWB without breaking backwards compatibility for ordered args to the NWBFile init. We switched everything to kwargs long ago and I'd be ok with breaking that at this point but that's the catch in making this optional.

For the GUIDE, I'd say let's make it required, matching the schema. Then let's consider making it optional in the schema separately.

CodyCBakerPhD commented 1 week ago

How does NeuroConv set session_description?

NeuroConv is the source of the current behavior, which makes it optional at a high level but because it's required to create a file, sets it to a default of an empty string (preferable to "no description" as happens automatically in a bunch of other places in NWB)

If the goal is to eventually make it optional to create a file, then it would be more work to make it temporarily required in GUIDE only to eventually undo that

Right now the momentum is still tilted in the direction of treating it as optional, so why not just take the next step and relax it at the schema level?