Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve documentation for some electrodes table columns #498

Merged
merged 10 commits into from
Apr 21, 2022

Conversation

rly
Copy link
Contributor

@rly rly commented Mar 5, 2022

Summary of changes

The rel_x, rel_y, rel_z, and reference columns of the electrodes table are confusing users.

And the documentation says the following about the reference column:
reference', 'Description of the reference used for this electrode.'
I fail to understand if this is meant to be an arbitrary string that we can use to reference the electrode of it is mean to work in tandem with rel_x, rel_y, rel_z and be the reference point form which they are measured.
Any pointers?

This PR clarifies the documentation string.

  • Update the PyNWB documentation accordingly.

PR checklist for schema changes

  • Add release notes for the PR to docs/format/source/format_release_notes.rst

@rly rly requested a review from bendichter March 5, 2022 01:54
rly added a commit to NeurodataWithoutBorders/pynwb that referenced this pull request Mar 5, 2022
@rly rly requested review from bendichter and removed request for bendichter April 20, 2022 06:51
@rly rly mentioned this pull request Apr 20, 2022
5 tasks
@bendichter
Copy link
Contributor

This change does not match with my understanding of this field. rel_x, y, and z and position on the probe. This should remain the same even if the probe is rotated in the brain, so it doesn't make sense to put this in terms of the anatomical coordinates of the brain.

@rly
Copy link
Contributor Author

rly commented Apr 20, 2022

@bendichter That makes sense. I will update the PR. Would it be useful to allow the user to specify an origin point / the reference frame for the rel_x, rel_y, and rel_z positions? I know for kilosort, the channel_positions.npy file stores what is basically rel_x and rel_y, but where the 0 point is is unclear (close to the tip? the connector?) and may not even be useful. Or perhaps we can punt this issue of reference frames until we have a better overall method for specifying spatial coordinates relative to one another.

@bendichter
Copy link
Contributor

@rly I think it would be good for us to try to converge with ProbeInterface on these types of questions.

bendichter
bendichter previously approved these changes Apr 20, 2022
@rly rly merged commit 4cc938f into dev Apr 21, 2022
@rly rly deleted the enh/improve_electrodes_docs branch April 21, 2022 21:16
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