reduce Images.jl dependency complexity

[johnnychen94]

I should have submitted this issue after I've thoroughly read source codes of all related packages, but I'd prefer throw this out and receive any possible feedback from the community.

According to my understanding, ImageCore is supposed to be an intermediate package between Color*, *Array, *View and Image* to hide the details. However, from the following graphs this "fact" is not so clear.

If this is the case, we might need to reduce the dependency of Image* to Color*,*Arrays and *Views to make the sense of layers clearer.

If we want to ease our developing work by hiding implementation details, then a naturally we should ask ourself:

• Should packages require Color*?
• Should packages require all kinds of *Arrays and *Views?

I'll keep this open and add more thoughts when I have.

Quote from Tim #766 (comment)

More generically, design-wise you're better off implementing the "high level" functions in terms of low-level operations that generalize across datatypes. If you load ColorVectorSpace, abs2 is ready-made for both numbers, grayscale, and AbstractRGB.

How I generate this graph

using Pkg
using Pkg.TOML: parsefile
using LightGraphs
using TikzGraphs
using TikzPictures
using Suppressor

const STDLIB_DIR = "/home/math/jc/.local/julia-1.1.0/share/julia/stdlib/v1.1"
const STDLIBS = readdir(STDLIB_DIR)

function get_dep_list(pkgname; exclude_list::Union{Nothing, Array} = nothing)
    tmp_dir = mktempdir(tempdir())
    Pkg.activate(tmp_dir)
    @suppress Pkg.add(pkgname)
    proj = parsefile(joinpath(tmp_dir, "Manifest.toml"));
    Pkg.activate()
    rm(tmp_dir,recursive=true)
    
    dep_list = []
    for (pkg, val) in proj
        val = val[1]
        # filter begin package
        (haskey(val, "deps") &&
         startswith(pkg, "Image") &&
         !(pkg in exclude_list)
        ) || continue
        
        for pkg_dep in setdiff(val["deps"], exclude_list)
            # filter end package
            startswith(pkg_dep, "Image") || 
            startswith(pkg_dep, "Color") || 
            endswith(pkg_dep, "Views") || 
            endswith(pkg_dep, "Arrays") ||
            endswith(pkg_dep, "Numbers") ||
            continue
            
            push!(dep_list, (pkg, pkg_dep))
        end
    end
    return dep_list
end

function build_dep_graph(dep_list)
    deps = Dict{String, Integer}()
    for (i, pkg) in enumerate(union(dep_list...))
        deps[pkg] = i
    end
    g = LightGraphs.SimpleGraphs.SimpleDiGraph(length(deps))

    for (i, (pkg, pkg_dep)) in enumerate(dep_list)
        add_edge!(g, deps[pkg], deps[pkg_dep])
    end
    return g
end

utils = ["Compat", "Pkg", "VersionParsing", "BinDeps", "BinaryProvider", "Requires", "Reexport",
         "AbstractFFTs", "FFTViews", "NaNMath",
         "URIParser", "JSON",
         "Missings"]
dep_list = get_dep_list("Images", exclude_list = vcat(utils, STDLIBS))
g = build_dep_graph(dep_list)
t = TikzGraphs.plot(g, Layouts.Layered(),options="scale=1, font=\\sf", union(dep_list...))

# TikzPictures.save(TikzPictures.SVG("graph"), t)
# TikzPictures.save(TikzPictures.PDF("graph"), t)

[johnnychen94]

To make Images a meta package, we need to move Metadata-related traits to subpackages.

I can think of two approaches here:

• move all traits in ImageMetadata. By doing this, ImageMetadata becomes another Images.jl
• move all traits to its method owner package, e.g., move restrict(img::ImageMeta, region::Dims) to ImageTransformations. Many packges will depend on ImageMetadata then.

A trick to reduce dependency is to use Requires.jl, but I heard that there's a performance issue on Requires.jl with julia 1.x, not quite sure how bad it is and if it's practical.

Any ideas?

[Tokazama]

If it's a trait I'd think ImageCore.jl would be the logical place for it. If it's a method then ImageAPI.jl would make sense to me. Just my two cents.

[timholy]

Echoing @johnnychen94, if we move all methods to ImageAPI.jl then we should save ourselves some time and just rename Images.jl to ImageAPI.jl 😛

I see three options:

1. investigate the Requires.jl concerns. Until we understand the concerns concretely, there is no point excluding what otherwise seems like a reasonable solution. improve callback performance JuliaPackaging/Requires.jl#64 appears to reduce load times, so this may be something that either has already been addressed or could be further improved.
2. make specific glue packages, e.g., ImageTransformationsMeta. This would have the advantage of permitting precompilation, once precompilation gets fixed (RFC: allow precompile to associate a MethodInstance with a module JuliaLang/julia#31466). Note this can be used in conjunction with Requires, as the @require command might just load this package.
3. reconsider the goal. IMO, the goal should be to improve user and/or developer experience. If that drives us to make Images.jl "just a metapackage," great, but I don't think that should become a goal on its own.

On the face of it, 1 (possibly in conjunction with 2) seems pretty reasonable. Taking the example of restrict, if you have AxisArray ranges encoding the spacing of the pixels, I'd almost call it a bug if restrict doesn't adjust the step size of the AxisArray ranges. Ideally, users shouldn't have to know that they can resolve this bug by manually loading another package, it should do the right thing all the time.

We didn't use Requires.jl previously because Requires.jl was a bit dicey before JuliaPackaging/Requires.jl#46.

[johnnychen94]

With #802 closed, here's the current status (*Arrays are removed from this graph):

[johnnychen94]

On the face of it, 1 (possibly in conjunction with 2) seems pretty reasonable.

If there's no objection, I would take this as the general principle. Precompilation of those codes might not give a significant boost speaking of loading time.

[johnnychen94]

close this as there's no specific issue remained.