Skip to content

Remove generic definitions of vectorplot! etc.#96

Merged
j-fu merged 2 commits intomainfrom
jf/fix/ambiguousvectorplot1d
Jul 9, 2025
Merged

Remove generic definitions of vectorplot! etc.#96
j-fu merged 2 commits intomainfrom
jf/fix/ambiguousvectorplot1d

Conversation

@j-fu
Copy link
Copy Markdown
Member

@j-fu j-fu commented Jul 9, 2025

They introduced dispatching ambiguities. Missing methods will throw an error anyway.

They introduced dispatching ambiguities. Missing methods
will throw an error anyway.
@j-fu j-fu requested a review from Copilot July 9, 2025 12:32
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes the generic “missing implementation” fallback methods for various plot! functions—letting Julia’s dispatch automatically throw a MethodError—and tightens the vectorplot! signatures by explicitly typing the ptype parameter.

  • Removed generic fallback definitions for gridplot!, scalarplot!, vectorplot!, and streamplot!
  • Updated vectorplot! method signatures to require ptype::Type{T} with a where constraint
Comments suppressed due to low confidence (3)

src/dispatch.jl:722

  • [nitpick] The parameter name ptype is ambiguous; consider renaming it to plotter_type for better readability and consistency.
function vectorplot!(ctx, ptype::Type{T}, ::Type{Val{1}}, grid, func::Matrix) where {T <: AbstractPlotterType}

src/dispatch.jl:725

  • [nitpick] Similarly, rename ptype to plotter_type here to match the updated signature and improve clarity.
function vectorplot!(ctx, ptype::Type{T}, ::Type{Val{1}}, grid, func::Vector) where {T <: AbstractPlotterType}

src/dispatch.jl:410

  • Since the generic fallback definitions were removed, add tests to verify that invoking gridplot!, scalarplot!, vectorplot!, and streamplot! without concrete implementations correctly raises a MethodError.
###################################################################################

thanks @copilot for the nitpick...
@j-fu j-fu merged commit 40656b8 into main Jul 9, 2025
9 checks passed
@j-fu j-fu deleted the jf/fix/ambiguousvectorplot1d branch July 9, 2025 15:18
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 this pull request may close these issues.

2 participants