{"action":"created","issue":{"url":"https://api.github.com/repos/scikit-image/scikit-image/issues/2646","repository_url":"https://api.github.com/repos/scikit-image/scikit-image","labels_url":"https://api.github.com/repos/scikit-image/scikit-image/issues/2646/labels{/name}","comments_url":"https://api.github.com/repos/scikit-image/scikit-image/issues/2646/comments","events_url":"https://api.github.com/repos/scikit-image/scikit-image/issues/2646/events","html_url":"https://github.com/scikit-image/scikit-image/issues/2646","id":225684483,"number":2646,"title":"Probable incorrect orientation use in EllipseModel","user":{"login":"jni","id":492549,"avatar_url":"https://avatars3.githubusercontent.com/u/492549?v=4","gravatar_id":"","url":"https://api.github.com/users/jni","html_url":"https://github.com/jni","followers_url":"https://api.github.com/users/jni/followers","following_url":"https://api.github.com/users/jni/following{/other_user}","gists_url":"https://api.github.com/users/jni/gists{/gist_id}","starred_url":"https://api.github.com/users/jni/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jni/subscriptions","organizations_url":"https://api.github.com/users/jni/orgs","repos_url":"https://api.github.com/users/jni/repos","events_url":"https://api.github.com/users/jni/events{/privacy}","received_events_url":"https://api.github.com/users/jni/received_events","type":"User","site_admin":false},"labels":[{"id":298491430,"url":"https://api.github.com/repos/scikit-image/scikit-image/labels/type:%20bug","name":"type: bug","color":"ea4041","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/scikit-image/scikit-image/milestones/7","html_url":"https://github.com/scikit-image/scikit-image/milestone/7","labels_url":"https://api.github.com/repos/scikit-image/scikit-image/milestones/7/labels","id":1646452,"number":7,"title":"0.14","description":"","creator":{"login":"soupault","id":1315589,"avatar_url":"https://avatars0.githubusercontent.com/u/1315589?v=4","gravatar_id":"","url":"https://api.github.com/users/soupault","html_url":"https://github.com/soupault","followers_url":"https://api.github.com/users/soupault/followers","following_url":"https://api.github.com/users/soupault/following{/other_user}","gists_url":"https://api.github.com/users/soupault/gists{/gist_id}","starred_url":"https://api.github.com/users/soupault/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/soupault/subscriptions","organizations_url":"https://api.github.com/users/soupault/orgs","repos_url":"https://api.github.com/users/soupault/repos","events_url":"https://api.github.com/users/soupault/events{/privacy}","received_events_url":"https://api.github.com/users/soupault/received_events","type":"User","site_admin":false},"open_issues":64,"closed_issues":174,"state":"open","created_at":"2016-03-15T19:16:06Z","updated_at":"2018-05-17T06:47:30Z","due_on":"2018-02-01T08:00:00Z","closed_at":null},"comments":12,"created_at":"2017-05-02T12:47:36Z","updated_at":"2018-05-17T12:26:42Z","closed_at":null,"author_association":"CONTRIBUTOR","body":"A user on the mailing list reported some weird behaviour by Ellipse Model:\r\n\r\nhttps://mail.python.org/pipermail/scikit-image/2017-April/005227.html\r\n\r\nIt looks like the angle is being estimated in the wrong direction. Here's some not-quite-minimal code to reproduce:\r\n\r\n```python\r\nimage = np.zeros((100, 100))\r\nd0 = np.diag(image, k=3)\r\nd0.flags.writeable = True\r\nd0[:] = 1\r\nd1 = np.diag(image, k=-3)\r\nd1.flags.writeable=True\r\nd1[:] = 1\r\nim[:20, :20] = 0\r\nimage[:20, :20] = 0\r\nimage[-20:, -20:] = 0\r\nimage[19, 17] = 1\r\nimage[18, 18] = 1\r\nimage[17, 19] = 1\r\nimage[82, 80] = 1\r\nimage[81, 81] = 1\r\nimage[80, 82] = 1\r\nfrom skimage.measure import EllipseModel\r\nell = EllipseModel()\r\ncnt = np.transpose(np.nonzero(image))\r\nell.estimate(cnt)\r\nxc, yc, a, b, theta = np.array(ellipse.params, dtype=float)\r\nprint(theta)\r\n```\r\nThe output is about π/4, which is correct if you rotate clockwise from the horizontal, rather than counterclockwise.\r\n\r\nIncidentally, the documentation for drawing the ellipse is also off, I think:\r\n\r\n```\r\norientation : double, optional\r\n    Major axis orientation in clockwise direction as radians.\r\n```\r\n\r\nBut drawing it seems to use a counter-clockwise theta rotation — unless you start from the vertical?\r\n\r\n```python\r\nrr, cc = draw.ellipse_perimeter(*np.round(ell.params[:4]).astype(int),\r\n                                theta)\r\nimage[rr, cc] = 0.67\r\nplt.imshow(image)\r\n```\r\n\r\n<img width=\"488\" alt=\"screen shot 2017-05-02 at 10 45 40 pm\" src=\"https://cloud.githubusercontent.com/assets/492549/25618156/1fb44522-2f89-11e7-9f25-bfd98a6eaec0.png\">\r\npurple: background\r\nyellow: image\r\ngreen: estimated ellipse.\r\n\r\nAnyway, it's a mess. =P"},"comment":{"url":"https://api.github.com/repos/scikit-image/scikit-image/issues/comments/389848635","html_url":"https://github.com/scikit-image/scikit-image/issues/2646#issuecomment-389848635","issue_url":"https://api.github.com/repos/scikit-image/scikit-image/issues/2646","id":389848635,"user":{"login":"soupault","id":1315589,"avatar_url":"https://avatars0.githubusercontent.com/u/1315589?v=4","gravatar_id":"","url":"https://api.github.com/users/soupault","html_url":"https://github.com/soupault","followers_url":"https://api.github.com/users/soupault/followers","following_url":"https://api.github.com/users/soupault/following{/other_user}","gists_url":"https://api.github.com/users/soupault/gists{/gist_id}","starred_url":"https://api.github.com/users/soupault/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/soupault/subscriptions","organizations_url":"https://api.github.com/users/soupault/orgs","repos_url":"https://api.github.com/users/soupault/repos","events_url":"https://api.github.com/users/soupault/events{/privacy}","received_events_url":"https://api.github.com/users/soupault/received_events","type":"User","site_admin":false},"created_at":"2018-05-17T12:26:42Z","updated_at":"2018-05-17T12:26:42Z","author_association":"MEMBER","body":"Ok, let me try to break this mess down into the isolated pieces:\r\n\r\n**Issue №1**. `ellipse` and `ellipse_perimeter` functions are implemented via different numerical algorithms. Because of this, `ellipse_perimeter` does not match exactly the hull of `ellipse`.\r\nA solution to this would be to rewrite the functions using the same numerical approach.\r\n\r\n**Issue №2**. `ellipse` and `ellipse_perimeter` have essentially different API. Specifically, `ellipse` takes `rotation` parameter and rotates the specified **ellipse** in CCW direction from Cartesian X-axis, while `ellipse_perimeter` takes `orientation` parameter and rotates the ellipse until the **major axis** lies in that direction CW. Obviously, the meanings of these parameters are very different. \r\nA solution to this is to decide on the desired API of these functions, unifying it as much as possible.\r\n\r\n**Issue №3**. `ellipse` and `ellipse_perimeter` parameter sequences are different, `r, c, rr, rc, shape, rot` and `r, c, rr, rc, \"rot\", shape` accordingly. Currently, it is a good thing, because users (probably) do not fall into thinking that `rotation` and `orientation` mean the same, but we have to make a decision on a unified API for both functions, and make the signatures as close to each other as possible.\r\n\r\n**Issue №4**. I'm pretty sure there are some issues related to the above in `EllipseModel` also, but it is mandatory first to converge on how we specify ellipse (coordinate conventions, rotation/orientation, refactor tests, etc), and only then review the estimation procedure.\r\n\r\nFinally, I personally feel that due to the inconsistency of  `ellipse` and `ellipse_perimeter` API and behaviour, the userbase of these functions is quite low. Thus, I'd propose to have a short (1 release) deprecation cycle for the old APIs of these functions, or maybe even no deprecation at all."}}
{"actor":{"display_login":"soupault"}}