Null values for properties are computed before checking if new rows need to be added

[h-mayorquin]

This issue is related to PR #1624 which exposes the null_values_for_properties parameter as a workaround for the error that occurs when properties lack sensible default values.

However, the underlying problem is that null values are computed preemptively without first checking whether they will actually be needed.

In add_electrodes_to_nwbfile, the code computes null values for all properties that exist in the electrode table but are not being added.

The error in _get_null_value_for_property is raised at this point if a property has a type without a sensible default.

However, these null values are only used later when adding new rows.

If channel_ids_to_add is empty (i.e., no new electrodes need to be added), the null values are never used, but the error would have already been raised.

The same pattern exists in _add_units_table_to_nwbfile.

[MGAMZ]

Continue the discussion in #1624

The bigger issue

I think the deeper issue you identified is real, and then I attempted to make relevant modifications to the code. First, you confirmed the information that needed to be injected, and then decided on the default value.

But unfortunately, I soon realized that in the problem scenario I encountered, channel_ids_to_add was not empty, so null values were necessary. I realized that the root cause of the problem I described should be: when processing the same source data, there is a difference in the logic for determining the electrode metadata values (e.g. Group Name) between NeuroConv spikeglxdatainterface and spiekinterface SpikeGLXRecordingExtractor.

The difference when handling SpikeGLX data

According to the NeuroConv SourceCode, the group_name will be f"Neuropixels{probe_name}", the channel_name will be f"AP{channel_number},LF{channel_number}".

According to the ProbeInterface SourceCode, contact_ids will be [f"e{elec_id}" for elec_id in elec_ids]. This value will soon be used by NeuroConv as electrode_name.

According to python-neo SourceCode, it follows the original meta name recorded in .meta file, which contains AP0,AP1....

Well, there're huge differences, which leads to _build_channel_id_to_electrodes_table_map generating a lot of None. This means all existing probes are not correctly recognized and the other probe definition with different naming rule will be writen again. Then, the null value issue is triggered.

In this situation, I think fundamentally standardizing the naming rules is risky and involves a considerable amount of work. So #1624 can still be a workaround, although there will exist duplicated electrode definitions...😑

[MGAMZ]

After I overrode the properties and aligned them with NeuroConv’s standard, I realized that the more significant issue you identified is essential to address.

According to your insight, my fix (#1633) avoids unnecessary null-value calculations because there are actually no rows to write to the electrode table.

With this change, the electrode-adding process can proceed successfully—even for recording with properties that lack sensible default values.

So my current conclusion is: The bug discribed in this issue is sensible and important. It makes my pipeline happy after #1633. 😃