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

Package breaks on DelaunayTriangulation 1.0, and question about code #12

Closed
DanielVandH opened this issue May 12, 2024 · 2 comments · Fixed by #13
Closed

Package breaks on DelaunayTriangulation 1.0, and question about code #12

DanielVandH opened this issue May 12, 2024 · 2 comments · Fixed by #13

Comments

@DanielVandH
Copy link
Contributor

DanielVandH commented May 12, 2024

The package currently breaks since on DelaunayTriangulation 1.0 indices has now been deleted. I'll get a PR in to fix it.

Also: Just curious, is there a particular reason why DelaunayTriangulation.jump_and_march was insufficient for the which_tri function? i.e. for

function which_tri(trivec::Vector{Vector{Vector{Float64}}}, x::Integer, y::Integer)
    findfirst(z -> 1 == inpolygon((x, y), trivec[z]), 1:length(trivec))
end

did jump_and_march have issues for you?

@DanielVandH DanielVandH mentioned this issue May 12, 2024
@jonocarroll
Copy link
Owner

Thank you! I suppose not having any tests meant that the green light on CI when merging the dependabot PR was somewhat meaningless - definitely my bad on that one.

I can't recall my exact reasoning when I wrote that function; I am new to this space and I was mostly trying to mimic scipy.spatial.Delaunay.find_simplex as I was adapting some existing python code.

Didn't you add jump_and_march in the last month or so? I may not have recognised it by this (or its former implementation's) name. I haven't fully explored other options for achieving this but one less dependency sounds great.

I'm checking out that PR now - many thanks!

@jonocarroll jonocarroll linked a pull request May 13, 2024 that will close this issue
@DanielVandH
Copy link
Contributor Author

jump_and_march has existed since the very beginning of the package almost. It is a bit of a bad name though I'd agree. It was originally called find_triangle but then I changed it for some reason (I think because I wanted to add some other methods for point location, which I never even did anyway...). I should change that next time I release a new version..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants