Skip to content

Conversation

@FrancescoNegri
Copy link
Contributor

@FrancescoNegri FrancescoNegri commented Mar 13, 2025

Hi, everyone!

I noticed that when converting a probe from probeinterface/Probe to ndx-probeinterface/Probe and then back to probeinterface/Probe some metadata (name, manufacturer, serial_number, model_name) are lost.

This is due to the to_probeinterface() method and I fixed it by assigning these attributes to the new probeinterface/Probe object.

More in general, annotations are not stored in the ndx-probeinterface/Probe object, thus they are not rewritten in the recovered probeinterface/Probe object.

Currently, this pull request fixes only the first issue concerning probe metadata, as they are saved as attributes of the ndx-probeinterface/Probe object. To my understanding, the retention of annotations in general would require some changes in the ndx-probeinterface/Probe class specification, making this task a bit more complex.

@alejoe91
Copy link
Member

Thanks @FrancescoNegri

I fixed the tests. Can you merge with main? It should make tests pass here as well

@FrancescoNegri
Copy link
Contributor Author

Sure @alejoe91! However, before merging, I wanted to fix the annotations issue as well.

Do you think that making a new attribute annotations for ndx-probeinterface/Probe might work?

@alejoe91
Copy link
Member

Sure @alejoe91! However, before merging, I wanted to fix the annotations issue as well.

Do you think that making a new attribute annotations for ndx-probeinterface/Probe might work?

Maybe :)

@FrancescoNegri
Copy link
Contributor Author

@alejoe91 I noticed you added an optional name parameter to from_probeinterface() and _single_probe_to_nwb_device() functions.
In case a probeinterface/Probe object has the name attribute set and the same parameter is specified in the from_probeinterface() function, then the probe instance attribute would be overwritten and lost. Wouldn't be better to make the name attribute of the probe compulsory instead of having an optional name parameter?

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.

2 participants