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

UndefVarError: triplot not defined #88

Closed
WalterMadelim opened this issue Sep 4, 2023 · 10 comments
Closed

UndefVarError: triplot not defined #88

WalterMadelim opened this issue Sep 4, 2023 · 10 comments

Comments

@WalterMadelim
Copy link

I'm encountering something which sounds weird.
I noticed your package yesterday (9/3/23) and use julia> ]add DelaunayTriangulation to install your package.
It turns out that the version installed is 0.7.2, according to the Project.toml inside the package.
At that version, the function triplot works properly, as your documention "Unconstrained Triangulation" suggests "fig, ax, sc = triplot(tri)". But I've been confused about how this function is included in the DelaunayTriangulation.jl, it seems to be inside this macro:

function triplot end # line 213, DelaunayTriangulation.jl

some lines here

@static if !isdefined(Base, :get_extension)
include("../ext/DelaunayTriangulationMakieCoreExt.jl")
end

some lines here

export triplot

However, It seems that get_extension is indeed defined in the Base module (I'm using win11+julia 1.9.3).
Thus it appears that the "include(***)" enclosed in the @static-end block should not be reached.
But this is not true right?, since you really wrote a line (352) that define this:

@doc (@doc _triplot) DelaunayTriangulation.triplot(args...; kwargs...) = _triplot(args...; kwargs...)

So I was confused from then on.
Until today I found some updates is available within my env, thus I use julia> ]up to update my packages.
After doing that I got the new version of your package (v 0.8.7).
But this time I get the error as mentioned in the title of this comment.
I find that you've removed this @static macro which made things involved.
But I clearly found that this triplot example is still in your documentation(v 0.8.7).

So could you help me? Thanks a lot!

@WalterMadelim
Copy link
Author

Sorry, I haven't been awared that I'm writing in md format.
the very big "some lines here" meant to be a comment in Julia code.
[with shame]

@DanielVandH
Copy link
Member

Hi @WalterMadelim: Hm. I thought the compat versions between Makie and DelaunayTriangulation would've forced this to still work. Do you have Makie loaded? You can check using

julia> ] st

(I removed triplot as of v0.8 since there were some circular dependency issues with Makie.jl - soon once Makie gets a new update, triplot will be provided by Makie.jl rather than here - see MakieOrg/Makie.jl#3159. Just waiting for a new version of Makie to get tagged so you can use v0.8 and Makie properly :).)

@WalterMadelim
Copy link
Author

Thank. I got your idea. Maybe you mean that the current developing project is not stable yet. I need to wait for a new release of Makie to cooperate with this package properly.

But I believe that the current version is not perfect:

(exp2) pkg> st
Status `K:\exp2\Project.toml`
⌃ [13f3f980] CairoMakie v0.10.6
  [927a84f5] DelaunayTriangulation v0.8.7
⌅ [ee78f7c6] Makie v0.19.6
  [860ef19b] StableRNGs v1.0.0
Info Packages marked with ⌃ and ⌅ have new versions available, but those with ⌅ are restricted by compatibility constraints from upgrading. To see why use `status --outdated`

julia> using DelaunayTriangulation, CairoMakie, StableRNGs
# some lines over here, without errors
julia> triplot!(ax, tri, show_convex_hull=true, show_ghost_edges=true)
ERROR: UndefVarError: `triplot!` not defined
Stacktrace:
 [1] top-level scope
   @ REPL[11]:1

As you see, I created a brand new env, adding your package (the default, the latest 0.8.7). And tried the example on your README.md, with the ERROR appearing. Actually if I did not install StableRNGs manually, the ERROR should be thrown at the 1st line of the example. So I think maybe this package is not self-contained. The ideal case might be: I create a new env, adding your package, then I can immediately start trying your examples out.

@DanielVandH DanielVandH mentioned this issue Sep 3, 2023
12 tasks
@DanielVandH
Copy link
Member

DanielVandH commented Sep 4, 2023

Ah. So yes the package is in a bit of an awkward state currently... I just tried it and you're right that the versions get messed up:

C:\Users\User>julia --project=-temp
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.9.3 (2023-08-24)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

(-temp) pkg> add DelaunayTriangulation, CairoMakie

(-temp) pkg> st
Status `C:\Users\User\-temp\Project.toml`
⌃ [13f3f980] CairoMakie v0.10.6
  [927a84f5] DelaunayTriangulation v0.8.7
Info Packages marked with ⌃ have new versions available and may be upgradable.

I thought it should force DelaunayTriangulation.jl to 0.7.2. In my testing, I must've been doing the updates one at a time, which does do what I want:

C:\Users\User>julia --project=-temp3
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.9.3 (2023-08-24)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

(-temp3) pkg> add CairoMakie

(-temp3) pkg> add DelaunayTriangulation

(-temp3) pkg> st
Status `C:\Users\User\-temp3\Project.toml`
  [13f3f980] CairoMakie v0.10.8
⌅ [927a84f5] DelaunayTriangulation v0.7.2
Info Packages marked with ⌅ have new versions available but compatibility constraints restrict them from upgrading. To see why use `status --outdated`

julia> using CairoMakie, DelaunayTriangulation

julia> tri = triangulate(rand(2, 50))
Delaunay Triangulation.
    Constrained: false
    Has ghost triangles: false
    Number of points: 50
    Number of triangles: 88
    Number of edges: 147

julia> triplot(tri) # works

Are you able to work with this for now? Meaning work with v0.7.2 (not much has changed, really) until Makie gets its new version? I didn't expect the new Makie version to take so long, sorry about this.

So I think maybe this package is not self-contained. The ideal case might be: I create a new env, adding your package, then I can immediately start trying your examples out.

There's a couple of things here. First: The package is self-contained in that, if all you want is a triangulation, you just need this package, e.g:

julia> tri = triangulate(rand(2, 50))
Delaunay Triangulation.
    Constrained: false
    Has ghost triangles: false
    Number of points: 50
    Number of triangles: 88
    Number of edges: 147

requires only DelaunayTriangulation. But for visualising it, you need a separate plotting package for that. In this case, Makie is that package. (It could be made to be self-contained by including Makie as a dependency, but that makes the package way too heavy.) This is a common way of doing things in other packages too. I should make this point clearer in the README. The line using DelaunayTriangulation, CairoMakie, StableRNGs I suppose should have a comment before it to first say that you need to add those packages.

For StableRNGs: I should probably remove that from the README example. It's there for reproducibility but it's not important there of course.

I am actually working now on revamping the documentation #86 to try and make this package a lot cleaner now that I've got some more time. I've added these points to my to-do list there.

I hope some of this makes it a bit clearer for you - appreciate the patience while the package is sadly in a bit of an awkward state momentarily. Let me know if you have more questions.

@DanielVandH
Copy link
Member

Realised that comments a bit long.. @WalterMadelim the gist is basically: Just use ]add DelaunayTriangulation@0.7.2 now to force the package to be at that version, and the package should be perfectly usable with Makie so that you can use triplot:

C:\Users\User>julia
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.9.3 (2023-08-24)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

(@v1.9) pkg> activate --temp
  Activating new project at `C:\Users\User\AppData\Local\Temp\jl_TkuCw0`

(jl_TkuCw0) pkg> add DelaunayTriangulation@0.7.2, CairoMakie

julia> using DelaunayTriangulation, CairoMakie

julia> triplot(triangulate(rand(2, 50)))

@WalterMadelim
Copy link
Author

Ok, I get your idea. Since you're manifestly an advanced developer, it's a sensible decision to keep this package compact.
The solution you offered resolves the problem, and returned to the stated I mentioned in my 1st comment.

Let me know if you have more questions.

Sure I have 1 question, which was talked in my 1st comment, if you are willing to answer.

using DelaunayTriangulation, CairoMakie
a = triangulate(rand(2,10))
println(@which triplot) # DelaunayTriangulation
triplot(a)

I wonder the calling process of the triplot function

function triplot end # line 213 in DelaunayTriangulation.jl
@static if !isdefined(Base, :get_extension)
    include("../ext/DelaunayTriangulationMakieCoreExt.jl")
end
export triplot

It seems that the "include("../ext/***")" here is where triplot is brought to DelaunayTriangulation.jl
But when I directly evaluate the condition following @static if, it should be false, thus how is the "include" involved?
I've been confused about this point, although not much related to the Delaunay Triangulation.
I know a little about macros, but not such familiar.

@DanielVandH
Copy link
Member

DanielVandH commented Sep 4, 2023

Those lines of code are there to support package extensions (https://pkgdocs.julialang.org/v1/creating-packages/#Weak-dependencies): In newer versions of Julia, when a package is loaded it can trigger some new code to load in the parent package (here, loading Makie would load the triplot code in DelaunayTriangulation by automatically including the code in ../ext/DelaunayTriangulationMakieCoreExt.j). For older versions of Julia, though (<1.9), package extensions don't exist, and so in that case I have to manually include the code in the package. This is done using those lines

@static if !isdefined(Base, :get_extension)
    include("../ext/DelaunayTriangulationMakieCoreExt.jl")
end

where:

  • @static allows the if statement to computed when the package is first loaded.
  • !isdefined(Base, :get_extension) checks if the current version of Julia has package extensions,

and the include just puts triplot into the package. So on your version of Julia, yes it will return false, but for versions <1.9, it will return true:

C:\Users\User>julia +1.9
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.9.3 (2023-08-24)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

julia> isdefined(Base, :get_extension)
true

julia> exit()

C:\Users\User>julia +1.8
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.8.5 (2023-01-08)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

julia> isdefined(Base, :get_extension)
false

@WalterMadelim
Copy link
Author

Thank you so much! Clear answers. I shall learn more about the Julia language and learn from the outstanding package developers, including you.

@DanielVandH
Copy link
Member

This should be all fixed now thanks to the new Makie release.

@WalterMadelim
Copy link
Author

Yes, you are right. It now resides in Makie.jl/src/basic_recipes/triplot.jl

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

No branches or pull requests

2 participants