search

exercism / julia

Exercism exercises in Julia.
https://exercism.org/tracks/julia
MIT License
66 stars 69 forks source link

Update circular-buffer example.jl #717

Closed depial closed 5 months ago

depial commented 5 months ago

Unwrapped the modules in example.jl in an attempt to resolve error occurring in Julia nightly - OS check suite.

Initial error read:

WARNING: both CircularBuffers and Base export "isfull"; uses of it in module anonymous must be qualified
When empty: Error During Test at /home/runner/work/julia/julia/exercises/practice/circular-buffer/runtests.jl:160
  Test threw exception
  Expression: isfull(cb) == false
  UndefVarError: `isfull` not defined in `Main.anonymous`
  Hint: It looks like two or more modules export different bindings with this name, resulting in ambiguity. Try explicitly importing it from a particular module, or qualifying the name with the module it should come from.
  Hint: a global variable of this name also exists in Base.

I am unaware of an isfull method in Base, however, for my first attempt at a fix, I tried to add change the isfull methods to Base.isfull, which fixed the Julia nightly - OS checks, but broke the others, with the following error message:

circular-buffer: Error During Test at /home/runner/work/julia/julia/runtests.jl:19
  Got exception outside of a @test
  LoadError: LoadError: UndefVarError: isfull not defined

Disposing off the module wrappers in example.jl seems to have fixed the issue. However, why this issue appeared should probably be looked into.

depial commented 5 months ago

All of the checks are passing now, but this fix is probably only a stop-gap. It would be great if @cmcaine could take a closer look at the underlying problem.

ErikSchierboom commented 5 months ago

@cmcaine I'm merging this to unblock CI

cmcaine commented 5 months ago

This fix is fine. The underlying issue is that Julia 1.11 exports an isfull function and that's clashing with the isfull function we're importing with the unqualified using expressions in example.jl (similar to from X import * in other languages).

We didn't encourage students to use modules or using, so this shouldn't cause issues for many students. I don't remember who wrote the example, but they're probably only in modules because I was writing them at a REPL (you can't redefine a struct, but you can redefine a module).

If we cared about keeping the modules (which I don't) then we could have fixed it by conditionally importing Base.isfull in each of the modules, as DataStructures.jl now does: https://github.com/JuliaCollections/DataStructures.jl/pull/896/files

# Base might define `isfull` from julia 1.11 onwards, see julia PR #53159
if isdefined(Base, :isfull)
    import Base: isfull
end