-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add circle model to the RANSAC gallery example #5208
base: main
Are you sure you want to change the base?
Conversation
Hello @sciunto! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2021-01-29 09:29:19 UTC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @sciunto. I have some minor comments, but this looks pretty good to me.
return x, y | ||
|
||
|
||
# Generate three types of data points |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you briefly describer what the purpose of these 3 are and which one we are expecting to estimate with the circle model?
xy = np.stack([x, y], axis=1).astype(float) | ||
|
||
# Fit with RANSAC | ||
ransac_kwarg = {'min_samples': 100, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not familiar with RANSAC, but looking at the output qualitatively I was wondering why only about half of the circle shows up as inliers in the right panel of the plot?
empirically it seems that changing min_samples
to a value <= 80 helps get many more inliers along the circle boundary.
ax[0].plot(x2, y2, '.') | ||
ax[0].plot(x1, y1, '.') | ||
|
||
circle = plt.Circle((cy, cx), radius=r, alpha=0.1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
increase alpha to 0.3 or so? I didn't really even see the disk at first!
ax[1].add_patch(circle) | ||
ax[1].plot(xy[inliers, 0], xy[inliers, 1], | ||
'bo', markersize=1, label='inliers') | ||
ax[1].plot(xy[~inliers, 0], xy[~inliers, 1], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
outliers
was defined above. Did you intend to use that here instead of ~inliers
?
Description
To investigate #5186 I wrote this example that I propose to add to the gallery.
Checklist
./doc/examples
(new features only)./benchmarks
, if your changes aren't covered by anexisting benchmark
For reviewers
later.
__init__.py
.doc/release/release_dev.rst
.