chengchingwen / Transformers.jl

Julia Implementation of Transformer models
MIT License
526 stars 75 forks source link

Name clash with `Base.stack` #116

Closed mcabbott closed 1 year ago

mcabbott commented 2 years ago

I went searching for uses of stack in packages and found this (on Julia nightly, i.e. 1.9-)

julia> using Transformers
WARNING: method definition for CuArray at /Users/me/.julia/packages/PrimitiveOneHot/M7M4C/src/gpu.jl:29 declares type variable A but does not use it.
WARNING: method definition for CuArray at /Users/me/.julia/packages/PrimitiveOneHot/M7M4C/src/gpu.jl:29 declares type variable N+1 but does not use it.
WARNING: method definition for CuArray at /Users/me/.julia/packages/PrimitiveOneHot/M7M4C/src/gpu.jl:29 declares type variable K but does not use it.

julia> @which Transformers.stack
WARNING: both Stacks and Base export "stack"; uses of it in module Transformers must be qualified
ERROR: "stack" is not defined in module Transformers

julia> @which Transformers.Stack.stack
getproperty(x::Type, f::Symbol)
     @ Base Base.jl:32

julia> methods(Transformers.Stacks.stack)
# 1 method for generic function "stack" from Transformers.Stacks:
 [1] stack(n, modeltype::Type{T}, args...; kwargs...) where T
     @ ~/.julia/packages/Transformers/bXd7C/src/stacks/stack.jl:40

julia> methods(Base.stack)
# 3 methods for generic function "stack" from Base:
 [1] stack(iter; dims)
     @ abstractarray.jl:2719
 [2] stack(f, iter; dims)
     @ abstractarray.jl:2748
 [3] stack(f, xs, yzs...; dims)
     @ abstractarray.jl:2749

https://github.com/chengchingwen/Transformers.jl/blob/7a7a25a3e29977678943b71aba0891ea934dcd62/src/stacks/stack.jl#L40

One mildly piratical quick-fix would be to make this Compat.stack(n::Int, modeltype::Type{T}, args...; kwargs...) where T = ..., since such a method never makes sense elsewhere.

Otherwise just using Stacks: stack in the main module will I think choose that over Base's?

chengchingwen commented 2 years ago

Actually, I don't think anyone is using that function... so we could probably just ignore that error. I would remove that function in the future.