Conversation
|
This pull request introduces 2 alerts when merging f4d6d3a into 56909cd - view on LGTM.com new alerts:
Comment posted by LGTM.com |
Codecov Report
@@ Coverage Diff @@
## master #6120 +/- ##
=========================================
- Coverage 89.04% 88.6% -0.45%
=========================================
Files 404 407 +3
Lines 73586 74160 +574
Branches 12218 12363 +145
=========================================
+ Hits 65525 65709 +184
- Misses 5188 5583 +395
+ Partials 2873 2868 -5 |
|
This pull request introduces 1 alert when merging 562c932 into 56909cd - view on LGTM.com new alerts:
Comment posted by LGTM.com |
|
This pull request introduces 1 alert when merging 0263807 into 56909cd - view on LGTM.com new alerts:
Comment posted by LGTM.com |
|
lol @ commit 0263807 |
|
Very cool to have this functionality! |
|
This pull request introduces 1 alert when merging 78a72f4 into 027f82d - view on LGTM.com new alerts:
Comment posted by LGTM.com |
|
@teonbrooks do you think we could make |
|
also don't forget to update whats_new.rst |
|
@teonbrooks sounds exciting! I hope we can get it merged soon |
|
@teonbrooks the CI is not happy! |
|
anyone with any idea why one of the travis builds is just becoming unresponsive? it's the one with py3.6 as req. am i doing something wrong with specifying the test build environment? |
f2e7e39 to
e61fe4a
Compare
|
The file |
|
so what I noticed it rewriting this tests several times is that the errorring is happening because of the incoming network notification Mac produce when I am launching the mock lsl stream. when running the scripts, the kernel doesn't get permission and then the client connection fails. I'm able to run the server when it is in interactive python, but not as a script |
change the pattern of checking for the import following common pattern seen in mne
forgot that I changed the class signature. If this doesn't work, I will implement multiprocessing to create the LSL Stream for the test
just caught the fact that I didn't change the base class to take the private connect function
Fix a typo in the LSLClient._connect which led to downstream errors.
jasmainak
left a comment
There was a problem hiding this comment.
After unrelenting pressure from @teonbrooks I had to approve this PR. But he has committed to improve things in the following PR #6141. This PR is already improving the state of things, so I don't want to block it any further.
@agramfort or @larsoner , if you are happy feel free to merge.
larsoner
left a comment
There was a problem hiding this comment.
Pushed a commit to add PyLSL to the requirements lists in two other places. Otherwise LGTM. Will merge once CIs come back happy.
Locally on Linux x86_64 Python 3.7 pip refused to install PyLSL>=1.13.1 saying:
Could not find a version that satisfies the requirement pylsl>=1.13.1 (from versions: 1.10.3, 1.10.4, 1.10.5, 1.12.2)
So I wouldn't be surprised if the 3.7-pip Linux x86_64 build also fails. This could be a packaging problem for PyLSL or a problem locally with my pip (or commands).
|
Indeed CircleCI, which is 3.7, confirms they have a packaging problem; @teonbrooks do you work with the PyLSL folks at all? Can you look into it? It is listed as supposedly compatible here: |
|
Actually looks like it's a known issue: I'll comment it out in |
|
Thanks @teonbrooks |
|
Is there an example where people can see how this LSL client works (ideally including online visualization of the incoming data)? |
|
@cbrnr, yes, it's part of #6141. @jasmainak didn't like the explicit call to generate a mock lsl stream ;). I didn't want a patched solution, so I made a proper mock stream class. I ran into a problem working with the RtEpochs class. I might need to split that investigation from the pending PR and tackle that next week at the sprint |
|
It's possible that the realtime code could use some fixes / cleanup, this would be great for the sprint |
my thoughts exactly. I opened an issue (#6157) and tacked it on to the Project board. |
|
I think it would be cool to emulate one of the FieldTrip examples with pylsl. So we can show the evoked update as the data comes in. |
|
Also, if we could have the realtime examples run in CircleCI that would be great. So that I don't have to run it on my computer every time. |
|
In theory we should be able to unify the structure for running a fake server from Travis / unit-tests for use by |
|
yes agreed. In the fieldtrip buffer, you can have a speedup factor so that it runs very fast. |
* working on the realtime client * new base class for rtclient * updates to the base client * add import * first pass at lsl in mne * UPDATE lsl client, base_client, FIX epochs * decouple the refactor from the LSLClient implementation this reverts the attempt to both add an LSLClient and create a BaseClient class. For this upcoming PR, the focus will be adding support to LSL. subsequent PRs can be done to refactor the realtime module * update to lsl client at the moment, I am able to connect to a stream. I need to test whether I am able to read the data from the stream. opened an issue on pylsl to inquire whether i can both send and receive on same machine for local dev purpose (labstreaminglayer/pylsl#10) once I can confirm the data is coming in properly, i will add some tests * Update lsl_client.py mistakenly tried to create inlets twice. this is the mvp for the LSL * code cleanup and the beginning of tests for the lslclient * Update test_lsl_client.py * starting to remember the pythonic way of coding been living in the world of javascript. feeling like a wayward youth * simplify time * Update lsl_client.py * address some comment in the pr change the pattern of checking for the import following common pattern seen in mne * [FIX] _testing.py: copy pasted but never corrected * Update test_lsl_client.py forgot that I changed the class signature. If this doesn't work, I will implement multiprocessing to create the LSL Stream for the test * testing is very useful just caught the fact that I didn't change the base class to take the private connect function * FIX arg in _connect; Implement thread to test Fix a typo in the LSLClient._connect which led to downstream errors. * address styling * replace thread with process for clean exit * linting complete * FIX: Add to two other req lists * FIX: Not on PyPi for Linux
|
FYI, I finally got around to updating the CI scripts so Appveyor automatically builds and distributes pylsl for all major platforms. There's a new version now on pypi. Please let me know if you encounter any problems. |
Got the urge to resurrect #4110 and maybe complete it. The aim of the PR is to create a realtime client for LSL that fits the way MNE handles Client objects. It also creates the basis for BaseClient class that will (in subsequent PRs) clean up the realtime module and make it easier to develop and use