-
Notifications
You must be signed in to change notification settings - Fork 190
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 an option to flip the order by depth #2005
Conversation
@@ -302,7 +302,7 @@ def get_chunk_with_margin( | |||
return traces_chunk, left_margin, right_margin | |||
|
|||
|
|||
def order_channels_by_depth(recording, channel_ids=None, dimensions=("x", "y")): | |||
def order_channels_by_depth(recording, channel_ids=None, dimensions=("x", "y"), flip=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe using the standard name reversed
from the python API would be a good idea?
https://docs.python.org/3/library/functions.html#sorted
It is also supported by the list.sort
But maybe you think that flip describes it better?
Just throwing something that I thought.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, on the other hand we have:
https://numpy.org/doc/stable/reference/generated/numpy.flip.html
Which seems to be aligned withat what you are doing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I started with reversed and finally change it to flip at the very last moment. I think flip is ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep. I also think that your docstring is doing the heavy lifting there anyway.
@@ -302,7 +302,7 @@ def get_chunk_with_margin( | |||
return traces_chunk, left_margin, right_margin | |||
|
|||
|
|||
def order_channels_by_depth(recording, channel_ids=None, dimensions=("x", "y")): | |||
def order_channels_by_depth(recording, channel_ids=None, dimensions=("x", "y"), flip=False): | |||
""" | |||
Order channels by depth, by first ordering the x-axis, and then the y-axis. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question out of curiosity, do we have a convention that x goes along the width or the recorder and that y goes in the height / depth (the direction where it is usualy moves) or is it the other way around?
I found myself realizing that I did not know what to expect after reading this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @h-mayorquin
Yes the convention is that x
is width and y
is depth. The ('x', 'y')
tuple is fed into lexsort
, which sorts outer to innermost dimensions ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @alejoe91.
No description provided.