{"action":"created","issue":{"url":"https://api.github.com/repos/ipython/ipykernel/issues/126","repository_url":"https://api.github.com/repos/ipython/ipykernel","labels_url":"https://api.github.com/repos/ipython/ipykernel/issues/126/labels{/name}","comments_url":"https://api.github.com/repos/ipython/ipykernel/issues/126/comments","events_url":"https://api.github.com/repos/ipython/ipykernel/issues/126/events","html_url":"https://github.com/ipython/ipykernel/pull/126","id":151093643,"number":126,"title":"General Fixes for Comms","user":{"login":"dwillmer","id":2343358,"avatar_url":"https://avatars.githubusercontent.com/u/2343358?v=3","gravatar_id":"","url":"https://api.github.com/users/dwillmer","html_url":"https://github.com/dwillmer","followers_url":"https://api.github.com/users/dwillmer/followers","following_url":"https://api.github.com/users/dwillmer/following{/other_user}","gists_url":"https://api.github.com/users/dwillmer/gists{/gist_id}","starred_url":"https://api.github.com/users/dwillmer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dwillmer/subscriptions","organizations_url":"https://api.github.com/users/dwillmer/orgs","repos_url":"https://api.github.com/users/dwillmer/repos","events_url":"https://api.github.com/users/dwillmer/events{/privacy}","received_events_url":"https://api.github.com/users/dwillmer/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"milestone":{"url":"https://api.github.com/repos/ipython/ipykernel/milestones/12","html_url":"https://github.com/ipython/ipykernel/milestones/4.4","labels_url":"https://api.github.com/repos/ipython/ipykernel/milestones/12/labels","id":1605410,"number":12,"title":"4.4","description":"backports for 4.4","creator":{"login":"minrk","id":151929,"avatar_url":"https://avatars.githubusercontent.com/u/151929?v=3","gravatar_id":"","url":"https://api.github.com/users/minrk","html_url":"https://github.com/minrk","followers_url":"https://api.github.com/users/minrk/followers","following_url":"https://api.github.com/users/minrk/following{/other_user}","gists_url":"https://api.github.com/users/minrk/gists{/gist_id}","starred_url":"https://api.github.com/users/minrk/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/minrk/subscriptions","organizations_url":"https://api.github.com/users/minrk/orgs","repos_url":"https://api.github.com/users/minrk/repos","events_url":"https://api.github.com/users/minrk/events{/privacy}","received_events_url":"https://api.github.com/users/minrk/received_events","type":"User","site_admin":false},"open_issues":1,"closed_issues":10,"state":"open","created_at":"2016-02-25T14:58:37Z","updated_at":"2016-04-28T13:45:29Z","due_on":null,"closed_at":null},"comments":1,"created_at":"2016-04-26T10:05:15Z","updated_at":"2016-04-28T13:54:56Z","closed_at":"2016-04-28T13:45:29Z","pull_request":{"url":"https://api.github.com/repos/ipython/ipykernel/pulls/126","html_url":"https://github.com/ipython/ipykernel/pull/126","diff_url":"https://github.com/ipython/ipykernel/pull/126.diff","patch_url":"https://github.com/ipython/ipykernel/pull/126.patch"},"body":"Howdy,\r\n\r\nThis is a general fix-up across the Comm and CommManager classes - there was quite a lot of redundant/duplicate code which has been cleaned up. There’s also a few performance items here - eg.  multiple identical dict lookups in the same function.\r\n\r\nI deliberately haven’t made whitespace changes, but the editor has auto-cleaned up some trailing whitespace.\r\n\r\nThe biggest changes (which need input from @minrk @takluyver) are the removal of `session` and `iopub_socket` from Comm and CommManager. The tests pass and i can't find any places where these items are used directly, but it could do with some thought from you.\r\n\r\nTheir use on CommManager was only to be passed into the Comm objects (which happened in 2 places), and their use on Comm objects is restricted to a single method, so it's simpler to just take them from the kernel object that needs passing down the hierarchy anyway. \r\n\r\nipykernel/comm/manager.py\r\n\r\n- [x] Removed lazy_keys function as it’s completely broken. There was only one use which was in a logging statement, and there was an error in the function definition because the single `dikt` argument wasn’t used. This would have caused a runtime exception had the code ever been hit. Have replaced the single use with a direct check of the logging level, which is more obviously correct to readers.\r\n- [x] Removed incorrect comment - `# call, because we store weak refs`. The item wasn’t called, and it’s not storing weak refs.\r\n- [x] Fixed get_comm to not perform the dict lookup twice.\r\n- [x] Removed session from CommManager. It’s not used internally, and the only instantiation of a comm manager i can find is on the kernel, in which case kernel.session is the correct, direct lookup.\r\n- [x] Reduced duplication of code between comm_msg and comm_close.\r\n- [x] Removed duplication of code between register_comm and comm_open. comm_open calls register_comm anyway, so theres no need to set the shell, kernel and iopub_socket twice.\r\n- [x] Removed iopub_socket, because with the previous change it’s only used internally in a single place, and there’s no need for the CommManager to know about it (can be taken directly from `kernel`. \r\n- [x] Removed shell, because with the previous changes it’s only used when being set on the comm objects, and they can just take it from the kernel object.\r\n- [x] Removed unused imports.\r\n- [x] Removed an EOL semi-colon.\r\n\r\nOverall these changes reduce the line count by about  20%, whilst also making the code more efficient, and easier to understand (i.e., smaller methods).\r\n\r\n\r\nipykernel/comm/comm.py\r\n\r\n- [x] Remove `shell` trait, because it’s only used in one method (and only then if a message callback is set), and there’s no need for the comm objects to know about it when that method can pull it off of `kernel`.\r\n- [x] Remove `iopub` for the same reason as `shell` above.\r\n- [x] Remove `session` for the same reason.\r\n\r\n\r\nipykernel/ipykernel.py\r\n\r\n- [x] Removed the redundant `shell` and `parent` args from being passed into CommManager."},"comment":{"url":"https://api.github.com/repos/ipython/ipykernel/issues/comments/215430850","html_url":"https://github.com/ipython/ipykernel/pull/126#issuecomment-215430850","issue_url":"https://api.github.com/repos/ipython/ipykernel/issues/126","id":215430850,"user":{"login":"dwillmer","id":2343358,"avatar_url":"https://avatars.githubusercontent.com/u/2343358?v=3","gravatar_id":"","url":"https://api.github.com/users/dwillmer","html_url":"https://github.com/dwillmer","followers_url":"https://api.github.com/users/dwillmer/followers","following_url":"https://api.github.com/users/dwillmer/following{/other_user}","gists_url":"https://api.github.com/users/dwillmer/gists{/gist_id}","starred_url":"https://api.github.com/users/dwillmer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dwillmer/subscriptions","organizations_url":"https://api.github.com/users/dwillmer/orgs","repos_url":"https://api.github.com/users/dwillmer/repos","events_url":"https://api.github.com/users/dwillmer/events{/privacy}","received_events_url":"https://api.github.com/users/dwillmer/received_events","type":"User","site_admin":false},"created_at":"2016-04-28T13:54:56Z","updated_at":"2016-04-28T13:54:56Z","body":"Thanks @minrk "}}