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

Require a method for specific subtypes only? #10

Open
BatyLeo opened this issue Sep 7, 2023 · 5 comments
Open

Require a method for specific subtypes only? #10

BatyLeo opened this issue Sep 7, 2023 · 5 comments
Labels
feature New feature

Comments

@BatyLeo
Copy link

BatyLeo commented Sep 7, 2023

I have a situation where I require a method only for some subtypes of the interface.

Would it be possible to support something like this for example?

abstract type MyType{T} end

function my_func end
function my_other_func end

@required MyType my_func(::MyType)

@required MyType{<:AbstractVector} my_other_func(::MyType)
@Seelengrab Seelengrab added the feature New feature label Sep 7, 2023
@Seelengrab
Copy link
Owner

This can get complicated very quickly. Should my_other_func error when it encounters a MyType{Int}, for example? This is not an issue in the current implementation, because @required doesn't handle UnionAll types right now. I'm inclined to think that they fall under the "optional interface" umbrella that IMO should be a distinct interface type, but I can also see how that is unnecessary duplication.

@BatyLeo
Copy link
Author

BatyLeo commented Sep 7, 2023

This can get complicated very quickly. Should my_other_func error when it encounters a MyType{Int}, for example?

In my case it shouldn't error, my_other_func is optional for MyType{Int}, it can exist but is not needed for using all implemented features of MyType. For instance, there can be a method my_feature in the package that uses my_other_func only when dispatched on a MyType{<:AbstractVector}.

Having the possibility to specify optional methods would be nice indeed.

Here is an extended example:

using RequiredInterfaces
using Test
const RI = RequiredInterfaces

abstract type MyType{T} end

function my_func end
function my_other_func end

@required MyType begin
    my_func(::MyType)
    my_other_func(::MyType)
end

function my_feature(t::MyType)
    return my_func(t)
end

function my_feature(t::MyType{<:AbstractVector})
    return my_func(t) * " " * my_other_func(t)
end

struct MySubType <: MyType{Int} end

my_func(t::MySubType) = "Hello"

struct MyOtherSubType <: MyType{Vector{Int}} end

my_func(t::MyOtherSubType) = "Hello"
my_other_func(t::MyOtherSubType) = "world"

struct MyWrongOtherSubType <: MyType{Vector{Int}} end

my_func(t::MyWrongOtherSubType) = "Hello"

The my_feature methods works as I want it to, which is nice:

my_feature(MySubType())   # returns "Hello"
my_feature(MyOtherSubType())   # returns "Hello world"
my_feature(MyWrongOtherSubType())   # raises a not implemented error, and suggests to implement my_other_func

However, check_interface_implemented does not work as I would want it to because my_other_func is considered mandatory, even for MySubType:

@test RI.check_interface_implemented(MyType, MyOtherSubType)   # passes
@test RI.check_interface_implemented(MyType, MyWrongOtherSubType)   # fails
@test RI.check_interface_implemented(MyType, MySubType)   # fails, and ideally should pass

@Seelengrab
Copy link
Owner

Yes, that is intentional - an interface cannot really have optional methods. In your example, I'd be open to add something like

abstract type MyType{T} end

@required MyType begin
    my_func(::MyType)
end

@required MyType{<:AbstractArray} begin
    my_other_func(::MyType{<:AbstractArray})    
end

or, with some syntax sugar:

@required T=MyType{<:AbstractArray} begin
    my_other_func(::T)    
end

being equivalent to

abstract type MyType end
# note how the UnionAll is resolved here by hand, and
# `MyTypeAbstractArray` inherits the interface of `MyType`
abstract type MyTypeAbstractArray <: MyType

@required MyType begin
    my_func(::MyType)
end

@required MyTypeAbstractArray begin
    my_other_func(::MyTypeAbstractArray)
end

but I wouldn't make that happen automatically and/or add my_other_func as an "optional" method to the MyType interface in general. It's not optional at all for MyType at all - it is required, and that's exactly the case when the type is <: MyType{<:AbstractArray}.

In other words, I'd say that no user of your package should ever expect or think of my_other_func as being "optional". Having that available on a type means it implements an entirely different (larger) type than can be assumed from MyType alone.

@BatyLeo
Copy link
Author

BatyLeo commented Sep 8, 2023

I didn't think of using a MyTypeAbstractArray inheriting from MyType, this works quite well in my case I think!
I also like your other suggestion, but it might not be needed in my case.

Thanks a lot for your help!

@Seelengrab
Copy link
Owner

Great! I'll leave this open, since support for UnionAll-style child interfaces seems intriguing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature
Projects
None yet
Development

No branches or pull requests

2 participants