Skip to content
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

skimage.transform.hough_line: new parameter to set bin size in the rho dimension #1748

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bennlich
Copy link
Contributor

From #1433:

it is possible to specify a custom angle range, but not a custom distance range. Is there a good reason why one can't stipulate a distance range.

Other implementations of the hough transform seem to use a "rho_resolution" parameter that represents how many bins to use in the rho dimension of the accumulator, so I thought I'd give that a try.

I'm new to Cython and C, and discovered that I can't use np.searchsorted in the with nogil block. Any suggestions for how I would go about doing something similar in C? I'm just trying to determine what bin index of the accumulator to increment (I think it should be pretty obvious if you take a look at my changes).

@bennlich
Copy link
Contributor Author

I think it would also make more sense to use a rho_resolution parameter that represents the width of a bin in the rho direction, rather than a num_rho parameter (as I've currently done), which represents the number of bins to use.

@stefanv
Copy link
Member

stefanv commented Oct 13, 2015

You cannot use numpy functions inside a with nogil block, because that takes you back to the Python layer (away from C). Have a look at my Cython tutorial if you want to know more: https://github.com/stefanv/teaching

@bennlich
Copy link
Contributor Author

@stefanv Yes, I figured that out. I'm wondering: what's a good way to accomplish np.searchsorted in C? (Or any better way of determining a bin index.)

@stefanv
Copy link
Member

stefanv commented Oct 13, 2015

You can make a call to searchsorted, you just have to do it outside with nogil. In the end, you'll have to time the new approach at what is already there.

Base automatically changed from master to main February 18, 2021 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants