Download correct ddl for 64 system on windows with Plexon #1350
Conversation
|
Hello @h-mayorquin! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2023-12-06 15:04:03 UTC |
There was a problem hiding this comment.
One question @h-mayorquin and I support the removal of the warning personally!
| # I think this warning should be removed | ||
| # Warnings should provide a solution but this is just a reminder to the user of normal behavior | ||
| # Nothing to do | ||
| warnings.warn(f'Using cached plexon dll at {pl2_dll_file_path}') |
There was a problem hiding this comment.
Super fair point. I don't think this warning is doing anything!
| @@ -288,7 +301,7 @@ def _get_signal_size(self, block_index, seg_index, stream_index): | |||
| stream_id = self.header['signal_streams'][stream_index]['id'] | |||
| stream_characteristic = list(self.signal_stream_characteristics.values())[stream_index] | |||
| assert stream_id == stream_characteristic.id | |||
| return stream_characteristic.n_samples | |||
| return int(stream_characteristic.n_samples) # Avoids returning a numpy.int64 scalar | |||
There was a problem hiding this comment.
This change is related to this:
SpikeInterface/spikeinterface#2223
There was a problem hiding this comment.
This should fix 3/4 of the pep8 issues. The line too long could be fixed, but then it would look super ugly. The L84 issue is the most annoying pep8 check ever. But there were spaces on the blank line, so I think I deleted them all.
EDIT: Just to be clear, I don't care about them but if they are enforced this is just to make it easier to fix them really quick.
Co-authored-by: Zach McKenzie <[email protected]>
Co-authored-by: Zach McKenzie <[email protected]>
Co-authored-by: Zach McKenzie <[email protected]>
Currently plexon download is fixed to use the 32 bits architecture. This means that plexon is not working if the windows machine is 64 bits. This PR fixes this by adding code that checks the architecture and selects the appropriate file to download based on that query.
There is a warning that I think should be removed in the code. Let me know what you think.
[EDIT] it seems that wine on linux uses the 32 bit architecture (I wonder why?) so I added code that restricts the 64 dll only for windows which does make sense anyway.