JuliaGeometry / JuliaGeometry/Contour.jl
Require z to be a matrix
Nobody has claimed this yet.
- Dominant language
- Julia
- Stars
- 46
- Forks
- 15
- PR merge metrics
- No merged PRs in 30d
Description
interpolate requires that z is an AbstractMatrix, ie rank 2, but contours and contour etc are happy to take any kind of data, as long as the first two dimensions match x and y.
I passed a rank 3 array (of eigenenergies over 2d momentum space), hoping to get contours over all the energies, but the error only appears in the interpolate function. Since z should be a matrix there, the higher-level functions should also only accept a matrix.
Weirdly, the MethodError isn't thrown if I pass a list of levels directly, or if those levels don't intersect with the data, because get_levels_cells returns an empty Dict, so the iteration in trace_contour doesn't happen and so the preallocated ContourLevel is returned without anything actually put in it:
https://github.com/JuliaGeometry/Contour.jl/blob/26994c47dcb9af88f2dc1961fb5291c8969afd1a/src/Contour.jl#L291
which relates to #80
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the higher-level contours and contour functions, then inspect interpolate and src/Contour.jl around line 291, including get_levels_cells and trace_contour. Reproduce the rank-3 input and the empty-level case, then add consistent validation so higher-level functions reject z values that are not matrices before contour tracing begins.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- julia
- Domain
- data-visualization
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100