-
Notifications
You must be signed in to change notification settings - Fork 261
Add plexon2 support #1214
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 plexon2 support #1214
Conversation
b043d4c
to
996a633
Compare
16c365f
to
693a496
Compare
This PR is in principle ready for review. Test files need to be merged for tests to pass. I didn't have any test files with multiple analog signal fragments present, if anybody has a file with this feature it would be great to be tested before merging. |
TODOs:
|
20001db
to
0393457
Compare
@samuelgarcia Tests are finally passing! Ready to merge if you are happy with it. |
This PR would be really instrumental in helping me (along with many other people I know) work with the PL2 files I received. @JuliaSprenger @samuelgarcia Could this be merged soon? :) |
if t_start is not None or t_stop is not None: | ||
# restrict spikes to given limits (in seconds) | ||
timestamp_frequency = self.pl2reader.pl2_file_info.m_TimestampFrequency | ||
lim0 = int(t_start * timestamp_frequency) | ||
lim1 = int(t_stop * self.pl2reader.pl2_file_info.m_TimestampFrequency) | ||
time_mask = (spike_timestamps >= lim0) & (spike_timestamps <= lim1) | ||
|
||
# limits are with respect to segment t_start and not to time 0 | ||
lim0 -= self.pl2reader.pl2_file_info.m_StartRecordingTime | ||
lim1 -= self.pl2reader.pl2_file_info.m_StartRecordingTime | ||
else: | ||
time_mask = slice(None, None) |
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.
To be safe here if we correct a bug one day I would share this piece of code with the one in _get_spike_timestamps
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 refactored and put the time masking into a dedicated utility method.
Hi julia. This is really hard to really give feedback of the ctype wrapper layer because I do not known the ddl. Maybe you could do some clean on the comments hinerited from exmaplerawio to keep only relevent ones. |
Hi Julia. |
Hi @samuelgarcia Thanks for the feedback. I addressed your comments and in principle only one is still open regarding loading of data by the channel name. I cleaned some of the comments that remained from the example io, but still many of them might be useful later on to understand why certain methods have certain return values and signatures. Can you check again and see if you are happy with the current changes? From my side this is ready for merging. |
OK lets go go for merging. |
if pl2_dll_file_path is None: | ||
pl2_dll_folder = pathlib.Path .home() / '.plexon_dlls_for_neo' | ||
pl2_dll_folder.mkdir(exist_ok=True) | ||
pl2_dll_file_path = pl2_dll_folder / 'PL2FileReader.dll' |
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 @JuliaSprenger
This pull request was super helpful!!
Just one thing, while executing Plexon2RawIO(). I got the error:
OSError: [WinError 193] %1 is not a valid Win32 application
and
OSError: Error: Can't load PL2FileReader.dll at: C:\Users\admin.plexon_dlls_for_neo\PL2FileReader.dll. PL2FileReader.dll is bundled with the C++ PL2 Offline Files SDK located on the Plexon Inc website: www.plexon.com Contact Plexon Support for more information: [email protected]
I then realized my pypl2 SDK folder had another dll file called "PL2FileReader64.dll"
so after replacing here "PL2FileReader.dll" with "PL2FileReader64.dll", it started running smoothly.
Just wanted to keep a note here if anyone else encounters those errors and wants to move ahead
This is a draft to add support for the new Plexon pl2 support to neo as among other sources requested in #1124
The RawIO will be based on the PyPl2 scripts and corresponding dlls provided by Plexon (see https://gitlab.com/penalab/pypl2 and https://github.com/juliasprenger/pypl2)
This PR requires the test file to be available on GIN, so https://gin.g-node.org/NeuralEnsemble/ephy_testing_data/pulls/97/files needs to be merged first.