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

WIP: Multidimensional structure tensor #2489

Conversation

tseclaudia
Copy link
Contributor

@tseclaudia tseclaudia commented Feb 5, 2017

This PR addresses issue #2346.

…n computing the BRIEF descriptors for normal and uniform mode
@tseclaudia tseclaudia changed the title WIP: WIP: Multidimensional structure tensor Feb 5, 2017
out = ()
derivatives = list(_compute_derivatives(image, mode=mode, cval=cval))
for i in range(len(derivatives)):
for j in range(len(derivatives)):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just use range(i, len(derivatives)) here and avoid the if statement on the next line. =)

Copy link
Member

@jni jni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tseclaudia I know this is a WIP but thought I would comment anyway:

  • don't comment out old code, just delete it. That's what git is for. =)
  • We probably want to use the "natural" order (ndi.sobel(..., axis=i) for i in range(image.ndim)) and reverse it only for 2D for backwards compatibility. See Update hessian matrix code to include order kwarg #2327 for an example.

Thanks for this!

@mkcor
Copy link
Member

mkcor commented Jul 17, 2021

It looks like this PR hasn't been active in a long time, plus #5002 superseded it. If there is no objection, I will close this PR [edit: by the end of] in the days following the sprint (SciPy US 2021).
/cc @tseclaudia @jni @coreysharris @emmanuelle @grlee77 @alexdesiqueira

@mkcor
Copy link
Member

mkcor commented Jul 20, 2021

Closing as per #2489 (comment)

@mkcor mkcor closed this Jul 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants