Skip to content
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

Make .get_data_as_epoch timeout a configurable parameter #49

Merged
merged 6 commits into from
Apr 14, 2023

Conversation

charlesbmi
Copy link
Contributor

As someone creating a real-time application, I sometimes want to limit the amount of time the application is waiting on data. This change makes wait_time (now called timeout) a configurable parameter.

In the situation that data is not ready before the timeout, .get_data_as_epoch now returns None because EpochsArray(data,...) errors out when data is an empty array.
Note: it was possible for data to be empty even before the change, although a shorter timeout now makes this more likely.

@larsoner
Copy link
Member

In the situation that data is not ready before the timeout, .get_data_as_epoch now returns None because EpochsArray(data,...) errors out when data is an empty array.

Should we change MNE-Python to allow EpochsArray to be an empty array of shape (0, n_channels, n_times)? We do allow instances of empty Epochs so I don't see why we couldn't do it for EpochsArray...

@charlesbmi
Copy link
Contributor Author

Should we change MNE-Python to allow EpochsArray to be an empty array of shape (0, n_channels, n_times)? We do allow instances of empty Epochs so I don't see why we couldn't do it for EpochsArray...

Seems reasonable to me! The specific error raised here is because the array is empty with shape (1, n_channels, n_times=0), but EpochsArray initialization also errors out if the passed in data is shape (0, n_channels, n_times) as you mention.

Separately, looks like the jobs are failing due to https://community.codecov.com/t/codecov-yanked-from-pypi-all-versions/4259/9

@larsoner
Copy link
Member

The update I pushed should fix the codecov problem

Feel free to make a PR to allow 0 length EpochsArray. Maybe not for no time points, but it seems reasonable for zero epochs

Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than one more numpydoc formatting thing, LGTM +1 for merge!

mne_realtime/lsl_client.py Outdated Show resolved Hide resolved
@larsoner larsoner enabled auto-merge (squash) April 14, 2023 15:23
@larsoner
Copy link
Member

Marking for merge when green @charlesincharge , thanks in advance!

@larsoner larsoner merged commit c7c3d9b into mne-tools:main Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants