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

Add get_channel_locations to the base recording api #3403

Merged

Conversation

h-mayorquin
Copy link
Collaborator

This will make the method appear on the BaseRecording API on the docs.

Plus, I added a docstring to the method.

No changes on behavior with this diff.

@h-mayorquin h-mayorquin added the documentation Improvements or additions to documentation label Sep 12, 2024
@h-mayorquin h-mayorquin self-assigned this Sep 12, 2024
Copy link
Collaborator

@zm711 zm711 left a comment

Choose a reason for hiding this comment

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

Small tweaks :)

src/spikeinterface/core/baserecording.py Outdated Show resolved Hide resolved
src/spikeinterface/core/baserecording.py Outdated Show resolved Hide resolved
axes: "xy" | "yz" | "xz" = "xy",
) -> np.ndarray:
"""
Get the physical locations of specified channels.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is likely improvement enough for merging, but since contact vs channel is still a point of confusion for many users we may want to workshop better ways to talk about channel_locations for public functions.

Copy link
Collaborator

@zm711 zm711 left a comment

Choose a reason for hiding this comment

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

Like I said my other point was more for us to think about long-term. This PR is a great doc improvement.

@alejoe91 alejoe91 merged commit b9f50e3 into SpikeInterface:main Sep 23, 2024
15 checks passed
@h-mayorquin h-mayorquin deleted the add_get_channel_locations_to_the_api branch September 23, 2024 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants