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

Curvilinear contourf #4744

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Curvilinear contourf #4744

wants to merge 4 commits into from

Conversation

asinghvi17
Copy link
Member

@asinghvi17 asinghvi17 commented Jan 20, 2025

Description

Fixes #796

This PR moves the contourf calculation outside the recipe so it can be dispatched on, and enables curvilinear contourf by transforming the output from Isoband using bilinear interpolation on the (x, y) grid.

@briochemc if you can make a PR against this branch that would be great! done!

Type of change

Delete options that do not apply:

  • New feature (non-breaking change which adds functionality)

Checklist

  • Added an entry in CHANGELOG.md (for new features and breaking changes)
  • Added or changed relevant sections in the documentation
  • Added unit tests for new algorithms, conversion methods, etc.
  • Added reference image tests for new plotting functions, recipes, visual options, etc.

@asinghvi17 asinghvi17 marked this pull request as draft January 20, 2025 01:29
return (polys, colors)
end

function Makie.plot!(c::Contourf{<:Union{<: Tuple{<:AbstractVector{<:Real}, <:AbstractVector{<:Real}, <:AbstractMatrix{<:Real}}, <: Tuple{<:AbstractMatrix{<:Real}, <:AbstractMatrix{<:Real}, <:AbstractMatrix{<:Real}}}})
Copy link
Member Author

Choose a reason for hiding this comment

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

Does anyone have an idea of how to clean this up? One way would be to go through convert_arguments and just error there but it's harder to do that for conversion traits.

Copy link
Member Author

Choose a reason for hiding this comment

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

Specifically I want to accept only (Vector, Vector, Matrix) or (Matrix, Matrix, Matrix) - at least for now

@MakieBot
Copy link
Collaborator

MakieBot commented Jan 20, 2025

Benchmark Results

SHA: 47f089c09a15121b09fb334d7a4722313dcd1dc2

Warning

These results are subject to substantial noise because GitHub's CI runs on shared machines that are not ideally suited for benchmarking.

GLMakie
CairoMakie
WGLMakie

@asinghvi17 asinghvi17 self-assigned this Jan 20, 2025
@asinghvi17 asinghvi17 added enhancement Feature requests and enhancements (tri)contour(f) not 3D contour, that's volume labels Jan 20, 2025
@asinghvi17 asinghvi17 changed the title [WIP] Curvilinear contourf Curvilinear contourf Jan 20, 2025
@asinghvi17 asinghvi17 marked this pull request as ready for review January 20, 2025 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests and enhancements (tri)contour(f) not 3D contour, that's volume
Projects
Status: Ready to review
Development

Successfully merging this pull request may close these issues.

Feature request: contour that can handle curvilinear grids
3 participants