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

Multiple Inheritance #43

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions BaseInterfaces/src/BaseInterfaces.jl
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ export Interfaces
export ArrayInterface, DictInterface, IterationInterface, SetInterface

include("interfaces/iteration.jl")
include("interfaces/dict.jl")
include("interfaces/set.jl")
include("interfaces/dict.jl")
include("interfaces/array.jl")

include("implementations/iteration.jl")
include("implementations/dict.jl")
include("implementations/set.jl")
include("implementations/dict.jl")
include("implementations/array.jl")

end
14 changes: 0 additions & 14 deletions BaseInterfaces/src/implementations/iteration.jl
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@

@implements IterationInterface{(:reverse,:indexing)} UnitRange [1:5, -2:2]
@implements IterationInterface{(:reverse,:indexing)} StepRange [1:2:10, 20:-10:-20]
@implements IterationInterface{(:reverse,:indexing)} Array [[1, 2, 3, 4], [:a :b; :c :d]]
@implements IterationInterface{(:reverse,:indexing)} Tuple [(1, 2, 3, 4)]
@static if VERSION >= v"1.9.0"
@implements IterationInterface{(:reverse,:indexing)} NamedTuple [(a=1, b=2, c=3, d=4)]
else
Expand All @@ -13,12 +8,3 @@ end
@implements IterationInterface Number [1, 1.0, 1.0f0, UInt(8), false]
@implements IterationInterface{:reverse} Base.Generator [(i for i in 1:5), (i for i in 1:5)]
# @implements IterationInterface{(:reverse,:indexing)} Base.EachLine [eachline(joinpath(dirname(pathof(BaseInterfaces)), "implementations.jl"))]

@implements IterationInterface Set [Set((1, 2, 3, 4))]
@implements IterationInterface BitSet [BitSet((1, 2, 3, 4))]
@implements IterationInterface Dict [Dict("a" => 2, :b => 3.0)]
@implements IterationInterface Base.EnvDict [Arguments(d=Base.EnvDict())]
@implements IterationInterface Base.ImmutableDict [Arguments(d=Base.ImmutableDict(:a => 1, :b => 2))]
# @implements IterationInterface IdDict
# @implements IterationInterface WeakKeyDict

2 changes: 1 addition & 1 deletion BaseInterfaces/src/interfaces/array.jl
Original file line number Diff line number Diff line change
Expand Up @@ -201,4 +201,4 @@ array_components = (;

_wrappertype(A) = Base.typename(typeof(A)).wrapper

@interface ArrayInterface AbstractArray array_components "Base Julia AbstractArray interface"
@interface ArrayInterface <: IterationInterface{(:reverse,:indexing)} AbstractArray array_components "Base Julia AbstractArray interface"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interface definitions are becoming fairly long to put everything on a single line.
I remember we had discussed defining the documentation somewhere else and just giving the macro a String variable, but for some reason it didn't work?

2 changes: 1 addition & 1 deletion BaseInterfaces/src/interfaces/dict.jl
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

@interface DictInterface AbstractDict ( # <: CollectionInterface
@interface DictInterface <: IterationInterface{(:reverse,)} AbstractDict ( # <: CollectionInterface
mandatory = (;
iterate = "AbstractDict follows the IterationInterface" => a -> Interfaces.test(IterationInterface, a.d; show=false) && first(iterate(a.d)) isa Pair,
eltype = "eltype is a Pair" => a -> eltype(a.d) <: Pair,
Expand Down
2 changes: 1 addition & 1 deletion BaseInterfaces/src/interfaces/set.jl
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,4 @@ set_components = (;
)
)

@interface SetInterface AbstractSet set_components "The `AbstractSet` interface"
@interface SetInterface <: IterationInterface AbstractSet set_components "The `AbstractSet` interface"
16 changes: 16 additions & 0 deletions BaseInterfaces/test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,22 @@ using Test
@implements SetInterface{(:empty,:emptymutable,:hasfastin,:intersect,:union,:sizehint!)} Test.GenericSet [Test.GenericSet(Set((1, 2)))]
@implements DictInterface Test.GenericDict [Arguments(d=Test.GenericDict(Dict(:a => 1, :b => 2)), k=:c, v=3)]

@testset "inheritance" begin
# Inherited interfaces are stored in the second parameter of the supertype
@test supertype(ArrayInterface) <: Interfaces.Interface{<:Any,IterationInterface{(:reverse, :indexing)}}
@test supertype(DictInterface) <: Interfaces.Interface{<:Any,IterationInterface{(:reverse,)}}
Comment on lines +11 to +12
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@test supertype(ArrayInterface) <: Interfaces.Interface{<:Any,IterationInterface{(:reverse, :indexing)}}
@test supertype(DictInterface) <: Interfaces.Interface{<:Any,IterationInterface{(:reverse,)}}
@test supertype(ArrayInterface) <: Interfaces.Interface{<:Any,>:IterationInterface{(:reverse, :indexing)}}
@test supertype(DictInterface) <: Interfaces.Interface{<:Any,>:IterationInterface{(:reverse,)}}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in case we define more interfaces in the future and the second parameter is a union, this seems more robust?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also why not just test ArrayInterface <: ...?


@test Interfaces.implements(ArrayInterface, Array)
@test Interfaces.implements(IterationInterface, Array)
@test !Interfaces.implements(DictInterface, Array)
@test !Interfaces.implements(SetInterface, Array)
@test Interfaces.implements(IterationInterface{(:reverse,:indexing)}, Array)

@test Interfaces.implements(IterationInterface, Dict)
@test !Interfaces.implements(IterationInterface{:indexing}, Dict)
@test !Interfaces.implements(ArrayInterface, Dict)
end

# Test all interfaces
@test Interfaces.test()

Expand Down
47 changes: 46 additions & 1 deletion src/implements.jl
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@
"""
function implements end
implements(T::Type{<:Interface}, obj) = implements(T, typeof(obj))
implements(::Type{<:Interface}, obj::Type) = false
implements(T::Type{<:Interface}, obj::Type) = inherits(T, obj)

function inherits end
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not qualified to review the exact implem but there are lots of codecov misses here

inherits(::Type{<:Interface}, obj) = false

"""
@implements(interface, objtype, test_objects)
Expand All @@ -39,6 +42,25 @@
macro implements(interface, objtype, test_objects)
_implements_inner(interface, objtype, test_objects)
end

inherited_type(::Type{<:Interface{<:Any,Inherits}}) where Inherits = Inherits
inherited_basetype(::Type{T}) where T = basetypeof(inherited_type(T))

inherited_optional_keys(::Type{<:Interface{Optional}}) where Optional = Optional
function inherited_optional_keys(::Type{T}) where T<:Union
map(propertynames(T)) do pn
inherited_optional_keys(getproperty(T, pn))

Check warning on line 52 in src/implements.jl

View check run for this annotation

Codecov / codecov/patch

src/implements.jl#L49-L52

Added lines #L49 - L52 were not covered by tests
end
end
inherited_optional_keys(::Type) = ()

Check warning on line 55 in src/implements.jl

View check run for this annotation

Codecov / codecov/patch

src/implements.jl#L55

Added line #L55 was not covered by tests

function inherited_interfaces(::Type{T}) where T <: Union
map(propertynames(T)) do pn
t = getproperty(T, pn)
inherited_optional_keys(t)

Check warning on line 60 in src/implements.jl

View check run for this annotation

Codecov / codecov/patch

src/implements.jl#L57-L60

Added lines #L57 - L60 were not covered by tests
end
end

function _implements_inner(interface, objtype, test_objects; show=false)
if interface isa Expr && interface.head == :curly
interfacetype = interface.args[1]
Expand All @@ -58,21 +80,44 @@
end
# Define a `implements` trait stating that `objtype` implements `interface`
$Interfaces.implements(::Type{<:$interfacetype}, ::Type{<:$objtype}) = true
# Define implements with user-specified `Options` to check
$Interfaces.implements(T::Type{<:$interfacetype{Options}}, O::Type{<:$objtype}) where Options =
$Interfaces._all_in(Options, $Interfaces.optional_keys(T, O))
# Define a method using `inherited_basetype` to generate the type that
# will dispatch when another Interface inherits this Interface.
function $Interfaces.inherits(::Type{T}, ::Type{<:$objtype}) where {T<:$Interfaces.inherited_basetype($interfacetype)}
implementation_keys = $Interfaces.inherited_optional_keys($Interfaces.inherited_type($interfacetype))
user_keys = $Interfaces._as_tuple($Interfaces._user_optional_keys(T))
return all(map(in(implementation_keys), user_keys))
end
# Define which optional components the object implements
$Interfaces.optional_keys(::Type{<:$interfacetype}, ::Type{<:$objtype}) = $optional_keys
$Interfaces.test_objects(::Type{<:$interfacetype}, ::Type{<:$objtype}) = $test_objects
nothing
end |> esc
end

_user_optional_keys(::Type{<:Interface{Options}}) where Options = Options
_user_optional_keys(::Type{<:Interface}) = ()

Check warning on line 101 in src/implements.jl

View check run for this annotation

Codecov / codecov/patch

src/implements.jl#L100-L101

Added lines #L100 - L101 were not covered by tests

_all_in(items::Tuple, collection) = all(map(in(collection), items))
_all_in(item::Symbol, collection) = in(item, collection)

struct Implemented{T<:Interface} end
struct NotImplemented{T<:Interface} end

function basetypeof(::Type{T}) where T
if T isa Union
types = map(propertynames(T)) do pn

Check warning on line 111 in src/implements.jl

View check run for this annotation

Codecov / codecov/patch

src/implements.jl#L111

Added line #L111 was not covered by tests
t = getproperty(T, pn)
getfield(parentmodule(t), nameof(t))
end
Union{types...}

Check warning on line 115 in src/implements.jl

View check run for this annotation

Codecov / codecov/patch

src/implements.jl#L115

Added line #L115 was not covered by tests
else
getfield(parentmodule(T), nameof(T))
end
end

"""
implemented_trait(T::Type{<:Interface}, obj)
implemented_trait(T::Type{<:Interface{Option}}, obj)
Expand Down
62 changes: 51 additions & 11 deletions src/interface.jl
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

Components is an `Tuple` of `Symbol`.
"""
abstract type Interface{Components} end
abstract type Interface{Components,Inherits} end

"""
optional_keys(T::Type{<:Interface}, O::Type)
Expand All @@ -21,6 +21,24 @@

mandatory_keys(T::Type{<:Interface}, args...) = keys(components(T).mandatory)

function _flatten_inheritance(::Type{T}) where T
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here for the codecov misses, is it due to macros?

t = if T isa Union
types = map(propertynames(T)) do pn
_flatten_inheritance(getproperty(T, pn))

Check warning on line 27 in src/interface.jl

View check run for this annotation

Codecov / codecov/patch

src/interface.jl#L24-L27

Added lines #L24 - L27 were not covered by tests
end
Union{T,types...}

Check warning on line 29 in src/interface.jl

View check run for this annotation

Codecov / codecov/patch

src/interface.jl#L29

Added line #L29 was not covered by tests
else
T

Check warning on line 31 in src/interface.jl

View check run for this annotation

Codecov / codecov/patch

src/interface.jl#L31

Added line #L31 was not covered by tests
end
return t

Check warning on line 33 in src/interface.jl

View check run for this annotation

Codecov / codecov/patch

src/interface.jl#L33

Added line #L33 was not covered by tests
end
function _flatten_inheritance(

Check warning on line 35 in src/interface.jl

View check run for this annotation

Codecov / codecov/patch

src/interface.jl#L35

Added line #L35 was not covered by tests
::Type{T}
) where T<:Interface{Options,Inherited} where {Options,Inherited}
t = Inherited <: Nothing ? T : Union{T,Inherited}
return t

Check warning on line 39 in src/interface.jl

View check run for this annotation

Codecov / codecov/patch

src/interface.jl#L38-L39

Added lines #L38 - L39 were not covered by tests
end

"""
test_objects(T::Type{<:Interface}, O::Type)

Expand Down Expand Up @@ -67,29 +85,51 @@
@interface MyInterface Any components description
```
"""
macro interface(interface::Symbol, type, components, description)
macro interface(interface_expr, type, components, description)
_error(interface_expr) = throw(ArgumentError("$interface_expr not recognised as an interface type."))

interface_expr = if interface_expr isa Symbol
interface_type = interface_expr
:(abstract type $interface_type{Components,Inherited} <: $Interfaces.Interface{Components,Nothing} end)
else
interface_expr.head == :<: || _error(interface_expr)
interface_type = interface_expr.args[1]
inherits_expr = interface_expr.args[2]
if inherits_expr isa Expr && inherits_expr.head == :curly
inherits_type = inherits_expr
:(abstract type $interface_type{Components,Inherited} <: $Interfaces.Interface{Components,$Interfaces._flatten_inheritance($inherits_type)} end)
elseif inherits_expr isa Symbol
inherits_type = inherits_expr
:(abstract type $interface_type{Components,Inherited} <: $Interfaces.Interface{Components,$inherits_type} end)

Check warning on line 103 in src/interface.jl

View check run for this annotation

Codecov / codecov/patch

src/interface.jl#L95-L103

Added lines #L95 - L103 were not covered by tests
else
_error(interface_expr)
end
end

quote
@assert $type isa Type
@assert $components isa NamedTuple{(:mandatory,:optional)}
@assert $description isa String
# Define the interface type (should it be concrete?)
abstract type $interface{Components} <: $Interfaces.Interface{Components} end
$interface_expr
# Define the interface component methods
$Interfaces.requiredtype(::Type{<:$interface}) = $type
$Interfaces.components(::Type{<:$interface}) = $components
$Interfaces.description(::Type{<:$interface}) = $description
$Interfaces.requiredtype(::Type{<:$interface_type}) = $type
$Interfaces.components(::Type{<:$interface_type}) = $components
$Interfaces.description(::Type{<:$interface_type}) = $description

Check warning on line 118 in src/interface.jl

View check run for this annotation

Codecov / codecov/patch

src/interface.jl#L118

Added line #L118 was not covered by tests
# Generate a docstring for the interface
let description=$description,
interfacesym=$(QuoteNode(interface)),
m_keys=$Interfaces.mandatory_keys($interface),
o_keys=$Interfaces.optional_keys($interface)
interface_sym=$(QuoteNode(interface_type)),
m_keys=$Interfaces.mandatory_keys($interface_type),
o_keys=$Interfaces.optional_keys($interface_type)
@doc """
$(" ") $interfacesym
$(" ") $interface_sym

An Interfaces.jl `Interface` with mandatory components `$m_keys` and optional components `$o_keys`.

$description
""" $interface
""" $interface_type
end
end |> esc
end

_namedtuple_expr_err() = error("components must be defined in-line in the macro with both `mandatory` and `optional` fields, not passed as a variable. E.g. (; mandatory=(; x=x_predicate), optional=(; y=y_predicate))")

Check warning on line 135 in src/interface.jl

View check run for this annotation

Codecov / codecov/patch

src/interface.jl#L135

Added line #L135 was not covered by tests
15 changes: 9 additions & 6 deletions src/test.jl
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,9 @@
test(T::Type{<:Interface}; kw...) =
_test_module_implements(Type{_check_no_options(T)}, nothing; kw...)

function _check_no_options(T)
T isa UnionAll || throw(ArgumentError("Interface options not accepted for more than one implementation"))
return T
end
_check_no_options(T::Type) = T
_check_no_options(::Type{<:Interface{Keys}}) where Keys =

Check warning on line 76 in src/test.jl

View check run for this annotation

Codecov / codecov/patch

src/test.jl#L76

Added line #L76 was not covered by tests
throw(ArgumentError("Interface options not accepted for more than one implementation"))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does that mean?

# Here we test all the `implements` methods in `methodlist` that were defined in `mod`.
# Basically we are using the `implements` method table as the global state of all
# available implementations.
Expand All @@ -99,7 +98,11 @@
t = b.parameters[2].var.ub
t isa UnionAll || return nothing, true

interface = t.body.name.wrapper
if t.body isa UnionAll
interface = t.body.body.name.wrapper
else
interface = t.body.name.wrapper

Check warning on line 104 in src/test.jl

View check run for this annotation

Codecov / codecov/patch

src/test.jl#L104

Added line #L104 was not covered by tests
end
implementation = b.parameters[3].var.ub
implementation == Any && return nothing, true

Expand Down Expand Up @@ -141,7 +144,7 @@
results = map(methodlist) do m
t = m.sig.parameters[2].var.ub
t isa UnionAll || return true
interface = t.body.name.wrapper
interface = t.body.body.name.wrapper
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recently discovered nameof, which might simplify this kind of stuff?

# If T implements it, test that
if implements(interface, T)
interface, test(interface, T; show, kw...)
Expand Down
Loading