ITK icon indicating copy to clipboard operation
ITK copied to clipboard

BUG: fix an infinite loop in itk::VoronoiDiagram2DGenerator

Open ILoveGoulash opened this issue 5 months ago • 5 comments

The loop exit condition was present in the code, but never wired and removed via https://github.com/InsightSoftwareConsortium/ITK/commit/cd97879e6bfec6cd9a33a3dd374d68cf4ff4303e

PR Checklist

  • [x] No API changes were made (or the changes have been approved)
  • [x] No major design changes were made (or the changes have been approved)
  • [ ] Added test (or behavior not changed)
  • [x] Updated API documentation (or API not changed)
  • [x] Added license to new files (if any)
  • [x] Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
  • [x] Added ITK examples for all new major features (if any)

Refer to the ITK Software Guide for further development details if necessary.

ILoveGoulash avatar Jun 16 '25 13:06 ILoveGoulash

Don't know if you want a regression test, and if your framework can handle "infinite looping" type failures.

ILoveGoulash avatar Jun 16 '25 13:06 ILoveGoulash

A regression test is advantageous. I don't think we have an explicit support for infinite looping, but such a test would fail via timeout.

dzenanz avatar Jun 16 '25 13:06 dzenanz

Added the original case by doing "monkey see monkey do", but it looks like I can't build locally due to IPFS + work firewall or something.

ILoveGoulash avatar Jun 16 '25 14:06 ILoveGoulash

@bradking Is ghostflow acting up? Commit 62457de has a very short commit message.

dzenanz avatar Jun 16 '25 14:06 dzenanz

Nah, the full "http://..." link is probably what triggers it, GitHub prettifies it on display.

ILoveGoulash avatar Jun 16 '25 14:06 ILoveGoulash

@ILoveGoulash Thank you for your contribution. There are just a few final actions that need to be finalized so that we can incorporate this into the main tree.

hjmjohnson avatar Aug 13 '25 15:08 hjmjohnson

Should be okay now.

ILoveGoulash avatar Aug 14 '25 07:08 ILoveGoulash

@ILoveGoulash I rebased and removed the merge conflict, then updated the commit message for the regression test to be more descriptive.

hjmjohnson avatar Aug 14 '25 12:08 hjmjohnson

Thanks!

ILoveGoulash avatar Aug 14 '25 13:08 ILoveGoulash