{"action":"created","issue":{"url":"https://api.github.com/repos/mne-tools/mne-python/issues/5712","repository_url":"https://api.github.com/repos/mne-tools/mne-python","labels_url":"https://api.github.com/repos/mne-tools/mne-python/issues/5712/labels{/name}","comments_url":"https://api.github.com/repos/mne-tools/mne-python/issues/5712/comments","events_url":"https://api.github.com/repos/mne-tools/mne-python/issues/5712/events","html_url":"https://github.com/mne-tools/mne-python/pull/5712","id":380699725,"node_id":"MDExOlB1bGxSZXF1ZXN0MjMwODM2Mzc4","number":5712,"title":"[WIP] Deprecate stim channel synthesis","user":{"login":"massich","id":7044835,"node_id":"MDQ6VXNlcjcwNDQ4MzU=","avatar_url":"https://avatars3.githubusercontent.com/u/7044835?v=4","gravatar_id":"","url":"https://api.github.com/users/massich","html_url":"https://github.com/massich","followers_url":"https://api.github.com/users/massich/followers","following_url":"https://api.github.com/users/massich/following{/other_user}","gists_url":"https://api.github.com/users/massich/gists{/gist_id}","starred_url":"https://api.github.com/users/massich/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/massich/subscriptions","organizations_url":"https://api.github.com/users/massich/orgs","repos_url":"https://api.github.com/users/massich/repos","events_url":"https://api.github.com/users/massich/events{/privacy}","received_events_url":"https://api.github.com/users/massich/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/mne-tools/mne-python/milestones/13","html_url":"https://github.com/mne-tools/mne-python/milestone/13","labels_url":"https://api.github.com/repos/mne-tools/mne-python/milestones/13/labels","id":3134180,"node_id":"MDk6TWlsZXN0b25lMzEzNDE4MA==","number":13,"title":"0.17","description":"","creator":{"login":"larsoner","id":2365790,"node_id":"MDQ6VXNlcjIzNjU3OTA=","avatar_url":"https://avatars1.githubusercontent.com/u/2365790?v=4","gravatar_id":"","url":"https://api.github.com/users/larsoner","html_url":"https://github.com/larsoner","followers_url":"https://api.github.com/users/larsoner/followers","following_url":"https://api.github.com/users/larsoner/following{/other_user}","gists_url":"https://api.github.com/users/larsoner/gists{/gist_id}","starred_url":"https://api.github.com/users/larsoner/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/larsoner/subscriptions","organizations_url":"https://api.github.com/users/larsoner/orgs","repos_url":"https://api.github.com/users/larsoner/repos","events_url":"https://api.github.com/users/larsoner/events{/privacy}","received_events_url":"https://api.github.com/users/larsoner/received_events","type":"User","site_admin":false},"open_issues":2,"closed_issues":102,"state":"open","created_at":"2018-02-21T18:43:16Z","updated_at":"2018-11-14T15:57:29Z","due_on":"2018-10-25T07:00:00Z","closed_at":null},"comments":2,"created_at":"2018-11-14T13:32:46Z","updated_at":"2018-11-14T16:21:48Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/mne-tools/mne-python/pulls/5712","html_url":"https://github.com/mne-tools/mne-python/pull/5712","diff_url":"https://github.com/mne-tools/mne-python/pull/5712.diff","patch_url":"https://github.com/mne-tools/mne-python/pull/5712.patch"},"body":"Stim channel is synthesized by:\r\n- [ ] brainvision\r\n- [ ] edf\r\n- [ ] eeglab"},"comment":{"url":"https://api.github.com/repos/mne-tools/mne-python/issues/comments/438723094","html_url":"https://github.com/mne-tools/mne-python/pull/5712#issuecomment-438723094","issue_url":"https://api.github.com/repos/mne-tools/mne-python/issues/5712","id":438723094,"node_id":"MDEyOklzc3VlQ29tbWVudDQzODcyMzA5NA==","user":{"login":"massich","id":7044835,"node_id":"MDQ6VXNlcjcwNDQ4MzU=","avatar_url":"https://avatars3.githubusercontent.com/u/7044835?v=4","gravatar_id":"","url":"https://api.github.com/users/massich","html_url":"https://github.com/massich","followers_url":"https://api.github.com/users/massich/followers","following_url":"https://api.github.com/users/massich/following{/other_user}","gists_url":"https://api.github.com/users/massich/gists{/gist_id}","starred_url":"https://api.github.com/users/massich/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/massich/subscriptions","organizations_url":"https://api.github.com/users/massich/orgs","repos_url":"https://api.github.com/users/massich/repos","events_url":"https://api.github.com/users/massich/events{/privacy}","received_events_url":"https://api.github.com/users/massich/received_events","type":"User","site_admin":false},"created_at":"2018-11-14T16:21:48Z","updated_at":"2018-11-14T16:21:48Z","author_association":"CONTRIBUTOR","body":"So far, I've deprecated a private function to see its ripples. These are the test that break.\r\n\r\n```\r\nResults (557.71s):\r\n     890 passed\r\n      19 failed\r\n         - mne/io/brainvision/tests/test_brainvision.py:100 test_vhdr_codepage_ansi\r\n         - mne/io/brainvision/tests/test_brainvision.py:135 test_ascii\r\n         - mne/io/brainvision/tests/test_brainvision.py:175 test_brainvision_data_highpass_filters\r\n         - mne/io/brainvision/tests/test_brainvision.py:233 test_brainvision_data_lowpass_filters\r\n         - mne/io/brainvision/tests/test_brainvision.py:288 test_brainvision_data_partially_disabled_hw_filters\r\n         - mne/io/brainvision/tests/test_brainvision.py:322 test_brainvision_data\r\n         - mne/io/brainvision/tests/test_brainvision.py:423 test_events\r\n         - mne/io/brainvision/tests/test_brainvision.py:595 test_brainvision_with_montage\r\n         - mne/io/brainvision/tests/test_brainvision.py:611 test_brainvision_neuroone_export\r\n         - mne/io/brainvision/tests/test_brainvision.py:643 test_read_raw_brainvision_warn_on_marker_drop\r\n         - mne/io/edf/tests/test_edf.py:59 test_orig_units\r\n         - mne/io/edf/tests/test_edf.py:132 test_edf_data\r\n         - mne/io/edf/tests/test_edf.py:184 test_stim_channel\r\n         - mne/io/edf/tests/test_edf.py:253 test_edf_annotations\r\n         - mne/io/edf/tests/test_edf.py:282 test_edf_stim_channel\r\n         - mne/io/edf/tests/test_edf.py:389 test_find_events_and_events_from_annot_are_the_same\r\n         - mne/io/eeglab/tests/test_eeglab.py:57 test_io_set_raw[fnames0]\r\n         - mne/io/eeglab/tests/test_eeglab.py:57 test_io_set_raw[fnames1]\r\n      37 skipped\r\n```\r\n\r\n## What I know so far:\r\n\r\nIn brainvision, the stim channel is always synthesized at `__init__`, and its only empty based on `event_id`. ( I also found that [this line](https://github.com/mne-tools/mne-python/blob/master/mne/io/brainvision/brainvision.py#L144) only allocates the stim channel and we might want to refactor it )\r\nhttps://github.com/mne-tools/mne-python/blob/master/mne/io/brainvision/brainvision.py#L170\r\n\r\nIn edf, we do can control the stim channel, by calling  `__init__` with `stim_channel=None` (right now defaults to `'auto'`).  \r\nhttps://github.com/mne-tools/mne-python/blob/master/mne/io/edf/edf.py#L346\r\n\r\neeglab is really similar to brainvision. The stim channel is always synthetized (it even have the same allocation).\r\nhttps://github.com/mne-tools/mne-python/blob/master/mne/io/eeglab/eeglab.py#L419\r\n\r\n\r\n---\r\n### The most conservative approach:\r\n\r\nIn 0.17:\r\n - add a parameter to compute stim_channel for brainvision and eeglab (`synthetize_stim_channel=True`)\r\n\r\nIn 0.18:\r\n - deprecate `synthetize_stim_channel=True` for brainvision and eeglab \r\n - deprecate `stim_channel='auto'` default in eeglab in favor of `None`\r\n\r\nIn 0.19:\r\n - Remove stim_channel\r\n\r\n### The most agressive approach:\r\nIn 0.17:\r\n - add a parameter to compute stim_channel for brainvision and eeglab (`synthetize_stim_channel=True`)\r\n - deprecate `synthetize_stim_channel=True` for brainvision and eeglab \r\n - deprecate `stim_channel='auto'` default in eeglab in favor of `None`\r\n\r\nIn 0.18:\r\n - Remove stim_channel\r\n"}}
{"actor":{"display_login":"massich"}}