Polygon filling API - Boundary Fill

[ashwani-rathee]

Initial setup of the Polygon filling API

• Implementation of Boundary Fill
  Linked issue :Polygon/Any closed path filling algorithms #53
  Demonstration : https://github.com/ashwani-rathee/Demonstrations/blob/main/boundary-fill-api.jl
  For some reason,the tests in poygon2d.jl isn't able to access boundaryfilll ,I am not sure on why as of now

[johnnychen94]

Don't rush, be patient, do less copy. Be careful about your format. Try to read, analyze and find out what the real issue is before asking for help.

I'm asking this because I hope I can spend more time reviewing and discussing more PR-related issues instead of those trivial things that you could have fixed by yourself.

[johnnychen94]

The test failure on Julia 1.0 should be fixed by #59 It's because pkg resolving in old Julia versions are not smart enough in some cases. (You can run git rebase master to update your branch)

Mostly it looks good to me now. Hopefully, this is the last round review.

If you're not confident about the implementation, you can always add more tests to make sure of it.

[johnnychen94]

@ashwani-rathee This PR a little bit long and old. Would you mind summarizing a little bit so that we can move forward quicker?

[ashwani-rathee]

@johnnychen94 In the latest improvements( 2 days back, before that were git rebase master changes), 12 tests were added and methods for cartesian index, point were added and bounds check was added for vertices and seed point.

[johnnychen94]

If I don't miss anything from previous comments, this should be the last round of review. Once it's fixed/answered, it should be good to merge.

You're starting to write good tests, that's good news 🎉 Do keep in mind: all reliable functionality is supported by more lines of codes of test cases.

[johnnychen94]

Oh boy, you can also change this to

Suggested change

	@boundscheck map(x -> (checkbounds(Bool, img, x[1], x[2]) ? nothing : throw(DomainError(x, "Vertice $x outside the image array domain"))), verts)
	@boundscheck foreach(x->checkbounds(img, x), verts)

[ashwani-rathee]

Yeah, I could have done that but that doesn't give information(in a concise way) to user what to correct 😕
Current expected error looks far more informative in my understanding I think.

    Expected: DomainError(CartesianIndex(0, 6), "Vertice CartesianIndex(0, 6) outside the image array domain")
      Thrown: BoundsError(ColorTypes.RGB[RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0); RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0); RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0); RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0); RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0); RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0); RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0) RGB{N0f8}(0.0,0.0,0.0)], (CartesianIndex(0, 6),))
Stacktrace: