Skip to content

Fixed bug in maxwellrawio.py that shuffled channels #1310

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jul 27, 2023
Merged

Fixed bug in maxwellrawio.py that shuffled channels #1310

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
28af3ec
Fixed bug in maxwellrawio.py that shuffled channels
hornauerp Jul 24, 2023
a0291d5
Merge pull request #1 from hornauerp/hornauerp-patch-1
hornauerp Jul 24, 2023
d97fe0c
More optimal bug fix
hornauerp Jul 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion neo/rawio/maxwellrawio.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ def _get_analogsignal_chunk(self, block_index, seg_index, i_start, i_stop,
# to be indexed out of order
sorted_channel_indexes = np.sort(channel_indexes)
resorted_indexes = np.array(
[list(channel_indexes).index(ch) for ch in sorted_channel_indexes])
[list(sorted_channel_indexes).index(ch) for ch in channel_indexes])
Copy link
Contributor

Choose a reason for hiding this comment

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

@hornauerp I think there is a better implementation.

Can you try if this gives the same result:

order_f = np.argsort(channel_indexes)
sorted_channel_idxeses = channel_indexes[order_f]
# use argsort again on order_f to obtain resorted_indexes
resorted_indexes = np.argsort(order_f)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, same outcome, but slightly faster. (17 vs 50 us)

I just wanted to stick to the original code as much as possible to facilitate the review process, but yours is definitely the more optimal implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great, can you use the second implementation then?

Let's refresh the code ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 👍


try:
if resorted_indexes is None:
Expand Down