-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Propagate any errors occurring within the _BaseClient
context
#46
Propagate any errors occurring within the _BaseClient
context
#46
Conversation
seems fair to me. Any possibility of adding a test? |
Thanks @jasmainak ! Added a test to check that the error gets propagated up to the user. |
…ser track down where an error came from.
16fb949
to
19320f2
Compare
Looks like this uncovered a real bug in our code/test:
maybe it just needs a |
…n_samples, but sfreq * n_secs * 2 is (often) a float
@larsoner I pushed 4c82e98 to fix that bug in After fixing the
(And then the test freezes and requires a FYI in case it matters, I am running on macOS. |
Sounds like it's sending more data than we need. In theory you should be able to tell it to mock a shorter file or set some timeout in a MNE-Realtime object to get it to timeout or something. See if there's anything like that, and if not I'll take a look. Really it shouldn't be your problem to have to fix so don't feel obligated to spend too much time on it... |
@charlesincharge You want @larsoner not me |
ah, sorry about that! |
3477d73
to
e439219
Compare
…nfinite generator, so it is not possible to convert to a list
e439219 should fix this error:
And then the test was subsequently froze on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, marking for merge when green. Thanks in advance @charlesincharge !
Current behavior: any Exceptions occurring within a
_BaseClient
context exit the context but do not propagate any errors.From https://docs.python.org/3/reference/datamodel.html#with-statement-context-managers about
__exit__
:(Note that
return self
acts as a true value here)New behavior: Exceptions are propagated through the context.
This helps user track down where an error came from.