search

msel-source / pymef

Python wrapper for MEF library
Other
5 stars 3 forks source link

Option to not crash the kernel if the password is wrong? #35

Closed tjr1 closed 1 year ago

tjr1 commented 1 year ago

Hi,

If the password to decrypt the data is wrong, the kernel crashes. Can we have it throw an error rather than crash the kernel? I'm trying to make a module for SpikeInterface and NWB that uses pymef. However, I don't think they will accept a pull request if we are crashing the kernel. Is the kernel crash something in the C meflib? Or can we fix this with a try except?

Thanks, Tom

xmival00 commented 1 year ago

I think changing this RuntimeError in the python code might be the way to go:

https://github.com/msel-source/pymef/blob/develop/pymef/mef_session.py#L191

Not sure what should be the logic of the pwd check though.

From my side, it might also be useful to enforce the way to go without pwd.

MaxvandenBoom commented 1 year ago

I think that line alone is unlikely to be the cause of a kernel crash.

@cimbi If you want, I can figure this one out with Tom and add another PR release (after https://github.com/msel-source/pymef/pull/34) to have it fixed in the new version.

tjr1 commented 1 year ago

Thanks for looking into it. I'm wondering if the kernel crash is due to something inside the C mef library...

MaxvandenBoom commented 1 year ago

@tjr1 Hey Tom,

I can't seem to make my kernel crash, for me it just stops with a 'RuntimeError: MEF password is invalid' as expected. Could you tell me what happens for you? Maybe with a screenshot or the lines you are using?

cimbi commented 1 year ago

I cannot crash the kernel either. We need to gather more information.

cimbi commented 1 year ago

@MaxvandenBoom I will not wait for this issue to be resolved because it can take a while and I will create a new pymef version. This one can be put in later on.

MaxvandenBoom commented 1 year ago

@cimbi Yeah, you're right. I also haven't been able to get a hold of Filip to figure out what he meant by "enforce the way to go without pwd", but I guess it can wait till a later release.

Could you make sure it also installs on python 3.11? (https://github.com/msel-source/pymef/issues/30#issuecomment-1648544774)

tjr1 commented 1 year ago

I'm so sorry. While it was repeatable a week ago, I can't get it do it anymore either. I'm not sure. I apologize for wasting your time. If it happens again, I'll try to sort it out.