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

BEP032: ephys -> {icephys, ecephys} #1806

Merged
merged 4 commits into from
Jun 5, 2024

Conversation

yarikoptic
Copy link
Collaborator

@yarikoptic yarikoptic commented Apr 25, 2024

Implements

Didn't want to push to bep032 since not yet agreed upon by @bids-standard/bep032 but wanted to be ready when/if we decide to proceed.

Closes #1800

@TheChymera
Copy link
Collaborator

Looks good to me.

TheChymera
TheChymera previously approved these changes Apr 29, 2024
@TheChymera
Copy link
Collaborator

Pending of course potential updates from the discussion.

@effigies
Copy link
Collaborator

@yarikoptic Do you intend to fix this up according to #1800 (comment) or should we close this?

@effigies effigies dismissed TheChymera’s stale review May 20, 2024 18:31

Consensus has changed

@yarikoptic
Copy link
Collaborator Author

let me fix it up to reflect #1800 ... will do now

@yarikoptic yarikoptic changed the title BEP032: ephys -> cephys BEP032: ephys -> {icephys, ecephys} May 20, 2024
…lectrode Electrophysiology

bids-standard#1800 (comment)

Consensus reached during working group meeting on 2024-05-15:

- modality = "Microelectrode Electrophysiology"
- datatypes = "icephys" and "ecephys"
- suffixes = "_icephys" and "_ecephys"
Copy link
Member

@VisLab VisLab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

microelectrodes -> microelectrode

@yarikoptic
Copy link
Collaborator Author

microelectrodes -> microelectrode

I might actually keep it plural and then make "cells" for intracel as well since there could be multiple electrodes/cells. Or why do you think it should become singular?

@yarikoptic
Copy link
Collaborator Author

I would also ask everyone to suggest using github suggestions mechanism, see e.g https://stackoverflow.com/questions/54239733/how-can-i-manually-add-suggestions-in-code-reviews-on-github on howto. Can even span multiple lines

@VisLab
Copy link
Member

VisLab commented Jun 3, 2024

It was grammar - "Electrophysiological data from an extracellular microelectrodes" -- the an is problematic if plural.

@yarikoptic
Copy link
Collaborator Author

It was grammar - "Electrophysiological data from an extracellular microelectrodes" -- the an is problematic if plural.

ah, indeed! codespell guru fell into a trap ;-)
I made two suggestions, see if I didn't screw up more: https://github.com/bids-standard/bids-specification/pull/1806/files#r1625124867

@yarikoptic
Copy link
Collaborator Author

FWIW -- pre-commit and check_links whining are not on changes in this PR

@yarikoptic
Copy link
Collaborator Author

ok, thanks everyone who participated -- let's proceed by merging into bep032 branch to avoid possible conflicts with other works. I will squash

@yarikoptic yarikoptic merged commit a46e439 into bids-standard:bep032 Jun 5, 2024
16 of 18 checks passed
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.

5 participants