{"action":"created","issue":{"url":"https://api.github.com/repos/napari/napari/issues/859","repository_url":"https://api.github.com/repos/napari/napari","labels_url":"https://api.github.com/repos/napari/napari/issues/859/labels{/name}","comments_url":"https://api.github.com/repos/napari/napari/issues/859/comments","events_url":"https://api.github.com/repos/napari/napari/issues/859/events","html_url":"https://github.com/napari/napari/pull/859","id":547147707,"node_id":"MDExOlB1bGxSZXF1ZXN0MzYwNjkwMTU2","number":859,"title":"Qt/Vispy connection & lambda refactor","user":{"login":"tlambert03","id":1609449,"node_id":"MDQ6VXNlcjE2MDk0NDk=","avatar_url":"https://avatars3.githubusercontent.com/u/1609449?v=4","gravatar_id":"","url":"https://api.github.com/users/tlambert03","html_url":"https://github.com/tlambert03","followers_url":"https://api.github.com/users/tlambert03/followers","following_url":"https://api.github.com/users/tlambert03/following{/other_user}","gists_url":"https://api.github.com/users/tlambert03/gists{/gist_id}","starred_url":"https://api.github.com/users/tlambert03/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tlambert03/subscriptions","organizations_url":"https://api.github.com/users/tlambert03/orgs","repos_url":"https://api.github.com/users/tlambert03/repos","events_url":"https://api.github.com/users/tlambert03/events{/privacy}","received_events_url":"https://api.github.com/users/tlambert03/received_events","type":"User","site_admin":false},"labels":[{"id":1238711279,"node_id":"MDU6TGFiZWwxMjM4NzExMjc5","url":"https://api.github.com/repos/napari/napari/labels/refactor","name":"refactor","color":"0e8a16","default":false,"description":"Refactoring code base"}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/napari/napari/milestones/2","html_url":"https://github.com/napari/napari/milestone/2","labels_url":"https://api.github.com/repos/napari/napari/milestones/2/labels","id":4298206,"node_id":"MDk6TWlsZXN0b25lNDI5ODIwNg==","number":2,"title":"0.2.0","description":"This milestone marks the addition of new features that will enable a large number of workflows within napari, including support for rapidly switching the displayed dimensions and projection type, support for 3D rendering, and support for multiple linked canvases. The aim is to have these features completed for the Bioimage Informatics Oct 2-4 2019 where napari will be featured in a software demo and talk.","creator":{"login":"sofroniewn","id":6531703,"node_id":"MDQ6VXNlcjY1MzE3MDM=","avatar_url":"https://avatars1.githubusercontent.com/u/6531703?v=4","gravatar_id":"","url":"https://api.github.com/users/sofroniewn","html_url":"https://github.com/sofroniewn","followers_url":"https://api.github.com/users/sofroniewn/followers","following_url":"https://api.github.com/users/sofroniewn/following{/other_user}","gists_url":"https://api.github.com/users/sofroniewn/gists{/gist_id}","starred_url":"https://api.github.com/users/sofroniewn/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sofroniewn/subscriptions","organizations_url":"https://api.github.com/users/sofroniewn/orgs","repos_url":"https://api.github.com/users/sofroniewn/repos","events_url":"https://api.github.com/users/sofroniewn/events{/privacy}","received_events_url":"https://api.github.com/users/sofroniewn/received_events","type":"User","site_admin":false},"open_issues":91,"closed_issues":203,"state":"open","created_at":"2019-05-09T01:17:28Z","updated_at":"2020-01-08T22:53:25Z","due_on":"2019-10-01T07:00:00Z","closed_at":null},"comments":0,"created_at":"2020-01-08T22:48:27Z","updated_at":"2020-01-09T02:20:03Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/napari/napari/pulls/859","html_url":"https://github.com/napari/napari/pull/859","diff_url":"https://github.com/napari/napari/pull/859.diff","patch_url":"https://github.com/napari/napari/pull/859.patch"},"body":"# Description\r\nFollowing up on the discussion in https://github.com/napari/napari/pull/846#discussion_r364016783, this PR is a broad sweep through the codebase cleaning up and standardizing our usage of connections (both in Qt and vispy), and lambda functions.    Here were the rules I followed:\r\n\r\n- if there is a class method that is more or less designed to be a vispy event handler or a qt slot, even if the method doesn't the event object, we should add `event=None` to the method signature so that we don't have to catch the event and discard it with a lambda every time it is connected:\r\n\r\n  ```python\r\n  self.events.something.connect(lambda x: self._on_something())\r\n  \r\n  #becomes\r\n  self.events.something.connect(self._on_something)\r\n  ...\r\n  def _on_something(self, event=None):\r\n      handle_something()\r\n  ```\r\n\r\n- `connect(lambda val: self.doSomething(val))` is the same as `connect(self.doSomething)` and has been changed where appropriate\r\n\r\n- defaults in lambda parameters are used in a lot of places throughout the code, but often the defaults don't have the same type as the eventual signal.  So it wasn't clear to me why they were being provided.  Removing them seems to have no effect either on tests or functionality, but let me know if I'm missing something:\r\n  ```python\r\n  sld.valueChanged.connect(lambda value=sld: self.changeSize(value))\r\n\r\n  # sld is never an integer, right? ... it becomes\r\n  sld.valueChanged.connect(self.changeSize)\r\n  ```\r\n\r\n- the syntax `widget.signal[type].connect` is used for [overloaded signals](https://wiki.qt.io/Qt_for_Python_Signals_and_Slots), and it is unnecessary unless the signal you are handling is overloaded _and_ you want the non default signature.  For instance: \r\n  ```python\r\n  # this is unnecessary because Qslider.valueChanged always returns an int\r\n  QSlider.valueChanged[int].connect()\r\n  # this is necessary because QCombBox.activated is overloaded, \r\n  # and the default is an int\r\n  QComboBox.activated[str].connect()\r\n\r\nI made a couple additional Qt refactors:\r\n\r\n- Juan's `QtModeButton` was being used in some places but other places had lots of duplicated classes.  I used his button class everyone and removed the duplicates.  But I renamed it to `QtModeRadioButton` because I also added a `QtModePushButton` that serves a similar code-reduction purpose.\r\n- wherever there was a `for` loop using `addItem` to a combobox, I turned it into `addItems`\r\n- I added a `keys()` method to our `StringEnum` because we often just want to get the list of options for a combo box.\r\n\r\ni think a lot of the code is much easier to follow now.  Hope you agree and hope it isn't a pain to merge into any open PRs.\r\n\r\n## Type of change\r\n<!-- Please delete options that are not relevant. -->\r\n- [x] Refactor\r\n\r\n# References\r\nhttps://wiki.qt.io/Qt_for_Python_Signals_and_Slots\r\n\r\n# How has this been tested?\r\n<!-- Please describe the tests that you ran to verify your changes. -->\r\n- [x] all tests pass with my change\r\n"},"comment":{"url":"https://api.github.com/repos/napari/napari/issues/comments/572350799","html_url":"https://github.com/napari/napari/pull/859#issuecomment-572350799","issue_url":"https://api.github.com/repos/napari/napari/issues/859","id":572350799,"node_id":"MDEyOklzc3VlQ29tbWVudDU3MjM1MDc5OQ==","user":{"login":"codecov[bot]","id":22429695,"node_id":"MDM6Qm90MjI0Mjk2OTU=","avatar_url":"https://avatars2.githubusercontent.com/in/254?v=4","gravatar_id":"","url":"https://api.github.com/users/codecov%5Bbot%5D","html_url":"https://github.com/apps/codecov","followers_url":"https://api.github.com/users/codecov%5Bbot%5D/followers","following_url":"https://api.github.com/users/codecov%5Bbot%5D/following{/other_user}","gists_url":"https://api.github.com/users/codecov%5Bbot%5D/gists{/gist_id}","starred_url":"https://api.github.com/users/codecov%5Bbot%5D/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/codecov%5Bbot%5D/subscriptions","organizations_url":"https://api.github.com/users/codecov%5Bbot%5D/orgs","repos_url":"https://api.github.com/users/codecov%5Bbot%5D/repos","events_url":"https://api.github.com/users/codecov%5Bbot%5D/events{/privacy}","received_events_url":"https://api.github.com/users/codecov%5Bbot%5D/received_events","type":"Bot","site_admin":false},"created_at":"2020-01-09T02:20:03Z","updated_at":"2020-01-09T02:20:03Z","author_association":"NONE","body":"# [Codecov](https://codecov.io/gh/napari/napari/pull/859?src=pr&el=h1) Report\n> Merging [#859](https://codecov.io/gh/napari/napari/pull/859?src=pr&el=desc) into [master](https://codecov.io/gh/napari/napari/commit/89d2f125afd14cd2bec696b90b277533b7aaa221?src=pr&el=desc) will **decrease** coverage by `0.13%`.\n> The diff coverage is `96.23%`.\n\n[![Impacted file tree graph](https://codecov.io/gh/napari/napari/pull/859/graphs/tree.svg?width=650&token=1LfdDeezHt&height=150&src=pr)](https://codecov.io/gh/napari/napari/pull/859?src=pr&el=tree)\n\n```diff\n@@            Coverage Diff             @@\n##           master     #859      +/-   ##\n==========================================\n- Coverage    84.1%   83.97%   -0.14%     \n==========================================\n  Files         167      168       +1     \n  Lines       16031    15894     -137     \n==========================================\n- Hits        13483    13347     -136     \n+ Misses       2548     2547       -1\n```\n\n\n| [Impacted Files](https://codecov.io/gh/napari/napari/pull/859?src=pr&el=tree) | Coverage Δ | |\n|---|---|---|\n| [napari/layers/shapes/shape\\_models/shape.py](https://codecov.io/gh/napari/napari/pull/859/diff?src=pr&el=tree#diff-bmFwYXJpL2xheWVycy9zaGFwZXMvc2hhcGVfbW9kZWxzL3NoYXBlLnB5) | `64.14% <ø> (ø)` | :arrow_up: |\n| [napari/\\_vispy/vispy\\_vectors\\_layer.py](https://codecov.io/gh/napari/napari/pull/859/diff?src=pr&el=tree#diff-bmFwYXJpL192aXNweS92aXNweV92ZWN0b3JzX2xheWVyLnB5) | `100% <100%> (ø)` | :arrow_up: |\n| [napari/\\_qt/layers/qt\\_base\\_layer.py](https://codecov.io/gh/napari/napari/pull/859/diff?src=pr&el=tree#diff-bmFwYXJpL19xdC9sYXllcnMvcXRfYmFzZV9sYXllci5weQ==) | `90.24% <100%> (-0.24%)` | :arrow_down: |\n| [napari/\\_vispy/vispy\\_points\\_layer.py](https://codecov.io/gh/napari/napari/pull/859/diff?src=pr&el=tree#diff-bmFwYXJpL192aXNweS92aXNweV9wb2ludHNfbGF5ZXIucHk=) | `89.23% <100%> (ø)` | :arrow_up: |\n| [napari/\\_vispy/vispy\\_image\\_layer.py](https://codecov.io/gh/napari/napari/pull/859/diff?src=pr&el=tree#diff-bmFwYXJpL192aXNweS92aXNweV9pbWFnZV9sYXllci5weQ==) | `93.1% <100%> (ø)` | :arrow_up: |\n| [napari/\\_vispy/vispy\\_shapes\\_layer.py](https://codecov.io/gh/napari/napari/pull/859/diff?src=pr&el=tree#diff-bmFwYXJpL192aXNweS92aXNweV9zaGFwZXNfbGF5ZXIucHk=) | `92% <100%> (ø)` | :arrow_up: |\n| [napari/utils/misc.py](https://codecov.io/gh/napari/napari/pull/859/diff?src=pr&el=tree#diff-bmFwYXJpL3V0aWxzL21pc2MucHk=) | `95.89% <100%> (+0.11%)` | :arrow_up: |\n| [napari/components/viewer\\_model.py](https://codecov.io/gh/napari/napari/pull/859/diff?src=pr&el=tree#diff-bmFwYXJpL2NvbXBvbmVudHMvdmlld2VyX21vZGVsLnB5) | `94.07% <100%> (ø)` | :arrow_up: |\n| [napari/\\_qt/layers/qt\\_image\\_base\\_layer.py](https://codecov.io/gh/napari/napari/pull/859/diff?src=pr&el=tree#diff-bmFwYXJpL19xdC9sYXllcnMvcXRfaW1hZ2VfYmFzZV9sYXllci5weQ==) | `93.51% <100%> (-0.06%)` | :arrow_down: |\n| [napari/\\_qt/qt\\_viewer\\_buttons.py](https://codecov.io/gh/napari/napari/pull/859/diff?src=pr&el=tree#diff-bmFwYXJpL19xdC9xdF92aWV3ZXJfYnV0dG9ucy5weQ==) | `87% <100%> (-3.23%)` | :arrow_down: |\n| ... and [15 more](https://codecov.io/gh/napari/napari/pull/859/diff?src=pr&el=tree-more) | |\n\n------\n\n[Continue to review full report at Codecov](https://codecov.io/gh/napari/napari/pull/859?src=pr&el=continue).\n> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)\n> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`\n> Powered by [Codecov](https://codecov.io/gh/napari/napari/pull/859?src=pr&el=footer). Last update [89d2f12...16b5581](https://codecov.io/gh/napari/napari/pull/859?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).\n"}}
{"actor":{"display_login":"codecov"}}