EEG-ExPy
EEG-ExPy copied to clipboard
refactor: upstreaming improved device classes from erb-thesis
Replaces #92 (switched to branch in the NeuroTechX repo instead of my fork)
Fixes #43
Fixes #77
TODO
- [ ] Resolve conflicts caused by changes to master since PR opened (that need to be integrated as part of a merge/rebase): https://github.com/NeuroTechX/eeg-notebooks/compare/ff5f018a9c9bb2c025a8c41d332ca20d371a7c59...master
- This was very helpful: https://stackoverflow.com/a/44956845/965332
- [ ] It's mostly changes in the
eeg.py
file that need attention (and whatever other files conflict)- Like the new _muse_get_recent method, and similar (probably best if whoever wrote them resolve the conflict, I feel very unsure...)
- [ ] Fix CI issues following merge with develop
- [ ] Test that things work
- [ ] For MuseDevice
- [ ] For BrainflowDevice
- [x] Ensure that tests and typechecking runs correctly in CI
- [ ] Check differences to
eeg.py
between fork and now- When I vendored the code into my thesis repo: https://github.com/ErikBjare/thesis/commit/fdaa9491ba522a076f16425772116430646706ac
- First change in
eegnb/devices
since I vendored the changes: https://github.com/NeuroTechX/eeg-notebooks/commit/b45201cb4937f4ab3c0c37667c955fd07a375e52- Full diff between that commit and now: https://github.com/NeuroTechX/eeg-notebooks/compare/b45201cb4937f4ab3c0c37667c955fd07a375e52...master
- Changes seem to mostly relate to the FreeEEG32
- [ ] Figure out how
util.py
is to be used in the new structure
@JadinTredup I got this PR in an almost mergeable state. The only thing left is to patch the FreeEEG32 stuff that was added after I forked eegnb/devices
(see top comment for links to diffs).
If we get the FreeEEG32 stuff fixes soon, I think we might actually be able to merge this soon.
I also added more testing (with pytest), which would be really nice to get merged. (oops, apparently I had similar changes in https://github.com/NeuroTechX/eeg-notebooks/pull/88)
Pushed an incomplete merge commit. Needs fixing (and careful review).
@JohnGriffiths @JadinTredup Looks like delaying this made the conflicts a lot worse... I tried to do my best to clean it up, but some issues left and it's 6PM here, so I'm off duty. Will take another look & continue tomorrow.
I would appreciate if you could both have a look and check if the general direction is good, and if the API looks reasonable (or if anything is unclear).