py_neuromodulation icon indicating copy to clipboard operation
py_neuromodulation copied to clipboard

Minor changes on top of mne_lsl merge

Open toni-neurosc opened this issue 1 year ago • 2 comments

Hi @timonmerk, I made some changes on top of the recent LSL merge. Summary:

  • Remove settings parameter from LSLOfflinePlayer since it's not used, as the sampling frequency is passed in another parameter
  • Defer LSLStream imports
  • Fix a typing error
  • Remove a couple variables
  • Changed a couple errors for warnings:
    • If both a filename and a data array was passed to LSLOfflinePlayer, it would throw an error. I changed it to a warning and defaulted to using the file as data source.
    • If _GenericStream was reading from an LSLStream with different frequency that the one that was passed it would throw an Exception, instead I think we can warn the user and default to the LSLStream frequency.

It's in general problematic to have 2 sources of truth like in the case of GenericStream getting an sfreq value from the user and one from the stream. It's also quite annoying that Python does not support overloaded constructors like C++. Maybe when I'm done with the Settings I will also incorporate Pydantic into the stream classes to handle this better.

toni-neurosc avatar May 08 '24 11:05 toni-neurosc

Thanks @toni-neurosc for that PR. Since there were a couple of changes in the branch refactor_lsl_stream (https://github.com/neuromodulation/py_neuromodulation/tree/refactor_lsl_stream) I would not directly merge it. Maybe you can check @SamedVossberg to include those lazy import changes in your next PR

timonmerk avatar May 14 '24 06:05 timonmerk

ok @timonmerk I have change this PR then to be a merge into the refactor_lsl_stream branch, at some point I will resolve the conflicts and let @SamedVossberg merge if he's alright with that

toni-neurosc avatar May 15 '24 08:05 toni-neurosc