Open rly opened 5 years ago
I'm fine with that as long as existing files with cached specifications still validate.
Units
should also be in nwb.ecephys.yaml.
Within nwb.ophys.yaml, a lot of types are defined within other types. e.g. PlaneSegmentation
is defined as a group within ImageSegmentation
. It's not clear to me why it should be defined there as opposed to as a more root location like most of the other types.
edit: I referred to ImagingPlane
but that is actually defined at the root level
I agree there isn't really a reason to define types in-line like that. I prefer root-level definitions as well
With the extraction of core types into hdmf-common-schema in #307, Device
into nwb.device.yaml
in #326, and removal of nested type definitions in #421, this issue is nearly resolved. The only remaining suggestions are to move NWBData
and NWBContainer
to nwb.core.yaml and Units
to nwb.ecephys.yaml. The former is not necessary. The latter is useful because intracellular units should use a different type (see #413) and having Units
(for extracellular units) in nwb.misc.yaml
(and in pynwb.misc
in PyNWB) which suggests a general-purpose use, would be confusing.
PyNWB contains both base.py and core.py. That organization seems logical. I propose doing the same for the YAML files. PyNWB also separates device.py into its own file. I propose doing the same for the YAML files for consistency.
Definitions that move from nwb.base.yaml to nwb.core.yaml:
NWBData
,Index
,VectorData
,VectorIndex
,ElementIdentifiers
,DynamicTableRegion
,NWBContainer
,NWBDataInterface
,DynamicTable
Definitions that move from nwb.file.yaml to nwb.device.yaml:
Device