`print_explicit_imports` suggests importing symbols from wrong module

[giordano]

With [email protected]:

julia> using Oceananigans, ExplicitImports

julia> print_explicit_imports(Oceananigans.OrthogonalSphericalShellGrids)
  Module Oceananigans.OrthogonalSphericalShellGrids is relying on implicit imports for 49 names. These could be explicitly imported as follows:

  [...]
  using LinearAlgebra: LinearAlgebra, /, convert
  [...]

but

julia> parentmodule(LinearAlgebra.:/)
Base

julia> parentmodule(LinearAlgebra.convert)
Base

Suggesting using / and convert from LinearAlgebra looks wrong?

[ericphanson]

huh, yeah, definitely seems weird... but I'm not totally sure ExplicitImports is wrong here:

julia> lookup = ExplicitImports.find_implicit_imports(Oceananigans.OrthogonalSphericalShellGrids)
Dict{Symbol, @NamedTuple{source::Module, exporters::Vector{Module}}} with 1318 entries:
  :Δr⁻¹ᶜᵃᶠ                                       => (source = Oceananigans.Operators, exporters = [Oceananigans.Operators])
  :Δy⁻¹ᵃᵃᶠ                                       => (source = Oceananigans.Operators, exporters = [Oceananigans.Operators])
  :weuclidean                                    => (source = Distances, exporters = [Distances])
  :ExponentialSpacing                            => (source = CubedSphere, exporters = [CubedSphere])
  :Ax⁻¹ᵃᶠᶠ                                       => (source = Oceananigans.Operators, exporters = [Oceananigans.Operators])
  :Eigen                                         => (source = LinearAlgebra, exporters = [LinearAlgebra])
  :Ax_qᶠᶜᶜ                                       => (source = Oceananigans.Operators, exporters = [Oceananigans.Operators])
  :MArray                                        => (source = StaticArraysCore, exporters = [StaticArrays])
  :Ayᶜᵃᶠ                                         => (source = Oceananigans.Operators, exporters = [Oceananigans.Operators])
  :Ayᶠᶠᶠ                                         => (source = Oceananigans.Operators, exporters = [Oceananigans.Operators])
  :Δx⁻¹ᵃᵃᵃ                                       => (source = Oceananigans.Operators, exporters = [Oceananigans.Operators])
  :cross                                         => (source = LinearAlgebra, exporters = [LinearAlgebra])
  :rank                                          => (source = LinearAlgebra, exporters = [LinearAlgebra])
  :Δx⁻¹ᶠᶠᶠ                                       => (source = Oceananigans.Operators, exporters = [Oceananigans.Operators])
  :ℑxyzᶜᶠᶜ                                       => (source = Oceananigans.Operators, exporters = [Oceananigans.Operators])
  :Δrᵃᶠᶜ                                         => (source = Oceananigans.Operators, exporters = [Oceananigans.Operators])
  :BetaPlane                                     => (source = Oceananigans.Coriolis, exporters = [Oceananigans])
  :Δλᶠᵃᶜ                                         => (source = Oceananigans.Operators, exporters = [Oceananigans.Operators])
  :SpecifiedTimes                                => (source = Oceananigans.Utils, exporters = [Oceananigans.Utils, Oceananigans])
  :Δy⁻¹ᶠᶠᶠ                                       => (source = Oceananigans.Operators, exporters = [Oceananigans.Operators])
  :Axᶠᶜᶜ                                         => (source = Oceananigans.Operators, exporters = [Oceananigans.Operators])
  :interior                                      => (source = Oceananigans.Fields, exporters = [Oceananigans])
  :rnode                                         => (source = Oceananigans.Grids, exporters = [Oceananigans.Grids])
  :ishermitian                                   => (source = LinearAlgebra, exporters = [LinearAlgebra])
  :NoFluxBoundaryCondition                       => (source = Oceananigans.BoundaryConditions, exporters = [Oceananigans.BoundaryConditions])
  :normalize                                     => (source = LinearAlgebra, exporters = [LinearAlgebra])
  :inject_halo_communication_boundary_conditions => (source = Oceananigans.DistributedComputations, exporters = [Oceananigans.DistributedComputations])
  ⋮                                              => ⋮

julia> lookup[:/]
(source = Base, exporters = Module[LinearAlgebra])

julia> Base.isexported(LinearAlgebra, :/)
true

julia> lookup[:convert]
(source = Base, exporters = Module[LinearAlgebra])

julia> Base.isexported(LinearAlgebra, :convert)
true

So we are recognizing the source is Base and they are exported by LinearAlgebra. They are also exported by Base:

julia> Base.isexported(Base, :/)
true

julia> Base.isexported(Base, :convert)
true

I guess there's a couple things here:

1. Why is LinearAlgebra re-exporting stuff from Base? seems weird
2. If you have using X and use names n exported from X, we suggest using X: n. This kind of makes sense, but if n is also exported from Base, it probably makes sense to just drop it? Special casing Base here. (Here X=LinearAlgebra, n = :convert).

[giordano]

If you have using X and use names n exported from X, we suggest using X: n.

Shouldn't it use parentmodule to find the module to suggest?

[ericphanson]

That’s the “source” module I mentioned. The problem is what if you depend on package Y which re-exports name from package X but you don’t depend on X? Y is saying name is part of its public api, and you depend on Y and want to use this. The fact that it’s defined in X doesn’t mean it’s the right module to import from; it may not be public there, or you might not depend on X.

[giordano]

The fact that it’s defined in X doesn’t mean it’s the right module to import from; it may not be public there

If Y makes it public but X doesn't, that looks dodgy to me on Y's side 😅

or you might not depend on X.

Yeah, I feel like that's a good indication you should depend on X.

[ericphanson]

It happens all the time though? Many "*Base" or "*Core" or whatever packages with higher level interfaces elsewhere. Or UUID, which is defined by Base but exported from UUIDs (https://github.com/JuliaTesting/ExplicitImports.jl/issues/29). If we wrote using Base: UUID then we are introducing semantic mistakes into the user's library because they are meant to do using UUIDs: UUID.