{"action":"created","issue":{"url":"https://api.github.com/repos/mne-tools/mne-python/issues/2765","labels_url":"https://api.github.com/repos/mne-tools/mne-python/issues/2765/labels{/name}","comments_url":"https://api.github.com/repos/mne-tools/mne-python/issues/2765/comments","events_url":"https://api.github.com/repos/mne-tools/mne-python/issues/2765/events","html_url":"https://github.com/mne-tools/mne-python/pull/2765","id":124775610,"number":2765,"title":"[WIP] Automatic info['nchan'] and info['ch_names']","user":{"login":"wmvanvliet","id":428273,"avatar_url":"https://avatars.githubusercontent.com/u/428273?v=3","gravatar_id":"","url":"https://api.github.com/users/wmvanvliet","html_url":"https://github.com/wmvanvliet","followers_url":"https://api.github.com/users/wmvanvliet/followers","following_url":"https://api.github.com/users/wmvanvliet/following{/other_user}","gists_url":"https://api.github.com/users/wmvanvliet/gists{/gist_id}","starred_url":"https://api.github.com/users/wmvanvliet/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/wmvanvliet/subscriptions","organizations_url":"https://api.github.com/users/wmvanvliet/orgs","repos_url":"https://api.github.com/users/wmvanvliet/repos","events_url":"https://api.github.com/users/wmvanvliet/events{/privacy}","received_events_url":"https://api.github.com/users/wmvanvliet/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":{"url":"https://api.github.com/repos/mne-tools/mne-python/milestones/8","html_url":"https://github.com/mne-tools/mne-python/milestones/0.12","labels_url":"https://api.github.com/repos/mne-tools/mne-python/milestones/8/labels","id":1429321,"number":8,"title":"0.12","description":"","creator":{"login":"Eric89GXL","id":2365790,"avatar_url":"https://avatars.githubusercontent.com/u/2365790?v=3","gravatar_id":"","url":"https://api.github.com/users/Eric89GXL","html_url":"https://github.com/Eric89GXL","followers_url":"https://api.github.com/users/Eric89GXL/followers","following_url":"https://api.github.com/users/Eric89GXL/following{/other_user}","gists_url":"https://api.github.com/users/Eric89GXL/gists{/gist_id}","starred_url":"https://api.github.com/users/Eric89GXL/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Eric89GXL/subscriptions","organizations_url":"https://api.github.com/users/Eric89GXL/orgs","repos_url":"https://api.github.com/users/Eric89GXL/repos","events_url":"https://api.github.com/users/Eric89GXL/events{/privacy}","received_events_url":"https://api.github.com/users/Eric89GXL/received_events","type":"User","site_admin":false},"open_issues":31,"closed_issues":9,"state":"open","created_at":"2015-11-25T13:36:18Z","updated_at":"2016-01-04T15:28:36Z","due_on":"2016-04-29T04:00:00Z","closed_at":null},"comments":11,"created_at":"2016-01-04T15:10:00Z","updated_at":"2016-01-05T11:59:55Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/mne-tools/mne-python/pulls/2765","html_url":"https://github.com/mne-tools/mne-python/pull/2765","diff_url":"https://github.com/mne-tools/mne-python/pull/2765.diff","patch_url":"https://github.com/mne-tools/mne-python/pull/2765.patch"},"body":"The info object has two redundant fields: `nchan` and `ch_names`. They are there as convenience fields. However, whenever the `chs` list is updated, these fields need to be manually updated as well.\r\n\r\nThis PR makes these fields behave more like properties.\r\n\r\nIt does so by making `Info` a subclass of `collections.MutableMapping`, which allows it to redefine `__setitem__` and `__getitem__` while retaining full compatibility with the default Python dict.\r\n\r\nThe `nchan` field just maps to `len(info['chs'])`.\r\n\r\nThe `ch_names` field is a bit more tricky. From the outside, it behaves as a mapping to `[ch['ch_name'] for ch in info['chs']]`. However, in order not to generate a new list every time the field is accessed, the field is an instance of `_ChannelNameList`.\r\n\r\nThe `_ChannelNameList` class is a subclass of `collections.Sequence`, thus implementing a list that is read-only, but otherwise fully compatible with a normal Python list. It overwrites the `__getitem(self, index)__` method to map to `info['chs'][index]['ch_name']` on the fly. It also defines a neat `make_index_map()` function that generates a dictionary that maps channel names to integer indices, which can be used to speed things up when many lookups are needed.\r\n\r\nThe rest of the code is updated to no longer set the `nchan` and `ch_names` fields of Info objects.\r\n\r\nCloses #2300 "},"comment":{"url":"https://api.github.com/repos/mne-tools/mne-python/issues/comments/168981814","html_url":"https://github.com/mne-tools/mne-python/pull/2765#issuecomment-168981814","issue_url":"https://api.github.com/repos/mne-tools/mne-python/issues/2765","id":168981814,"user":{"login":"jasmainak","id":15852194,"avatar_url":"https://avatars.githubusercontent.com/u/15852194?v=3","gravatar_id":"","url":"https://api.github.com/users/jasmainak","html_url":"https://github.com/jasmainak","followers_url":"https://api.github.com/users/jasmainak/followers","following_url":"https://api.github.com/users/jasmainak/following{/other_user}","gists_url":"https://api.github.com/users/jasmainak/gists{/gist_id}","starred_url":"https://api.github.com/users/jasmainak/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jasmainak/subscriptions","organizations_url":"https://api.github.com/users/jasmainak/orgs","repos_url":"https://api.github.com/users/jasmainak/repos","events_url":"https://api.github.com/users/jasmainak/events{/privacy}","received_events_url":"https://api.github.com/users/jasmainak/received_events","type":"User","site_admin":false},"created_at":"2016-01-05T11:59:55Z","updated_at":"2016-01-05T11:59:55Z","body":"> I've spend some days now digging through the code, hunting down all instances where ch_names is used. We cannot easily drop info['ch_names']. It's super useful and used extremely frequently in the code.\r\n\r\nWhy is it difficult to simply replace every instance of info['ch_names'] with info.ch_names? Or do I miss something here?\r\n\r\n> Any substantial change to it breaks things in nearly every file. Debugging the necessary refactoring is not going to be fun. Not to mention it will break many user scripts as well. \r\n\r\nIt's already an API change. Won't this also break user scripts -- to make it read only? Also imagine how many users would be trying to use this pattern:\r\n\r\n```py\r\nch_names = info['ch_names']\r\nch_names[5] = 'abc' # or some manipulation for their own purposes\r\n```\r\nNow this is broken. I don't mind the fact that this doesn't work -- but the error trace you get is cryptic. Making it a property will make it clear to the user that this is something they are not supposed to manipulate. Again, why is `info['bads']` a list? It's not consistent.\r\n\r\n> As I see it, some classes in MNE work as a dictionary. Namely the old C-structures lifted from MNE-C like Info, ForwardOperator and InverseOperator. Some classes work as an object. Namely the ones that were written from scratch like Evoked, SourceEstimate and ICA. In a perfect world, I would like to see everything using normal object syntax (info.ch_names, info.chs, info.nchan, info.sfreq) but that would be a huge change that breaks everything. In a less perfect world I would like to see a strict separation between the two 'styles' of classes. So for me a -1 on adding property methods to Info; having to remember whether a class wants dictionary style access or object style access is hard enough without having to remember this for each field.\r\n\r\nSo, your main complaint is that it's too much effort? I'm worried that what you're proposing is not much better than what we already have (except maybe some speed gains) in terms of avoiding errors. In fact, it will lead to cryptic error messages. Also, I tried running an example on your branch and it's broken. So, you'll have to check the examples too to make sure you don't break them. \r\n"}}