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

API for interface tests #2

Open
lorenzoh opened this issue Jun 1, 2022 · 5 comments
Open

API for interface tests #2

lorenzoh opened this issue Jun 1, 2022 · 5 comments

Comments

@lorenzoh
Copy link
Owner

lorenzoh commented Jun 1, 2022

Continuing the discussion from https://discourse.julialang.org/t/discussion-on-why-i-no-longer-recommend-julia-by-yuri-vishnevsky/81151/219?u=holylorenzo @rafaqz.

Yeah lets try and get as much as possible out of defining this list! Maybe to make it available for other uses, instead of calling check like that, you could call it on the object then call the implements trait:

implements(::Type{<:MyObject}) = my_interface(optional_thing_1=true, optional_thing_2=false)

then in tests do:

check(obj)

and in docs like this would generate the markdown list of implemented traits and sub-traits:

@implements MyObject

and then in Invariants.jl define check like:

check(obj) = check(implements(obj), obj))
etc.

One problem I see with implements returning a list of invariants is that now other packages cannot extend the list of supported interfaces for a type. I think that specifically dispatching on an interface that should be checked would resolve that.

@rafaqz
Copy link

rafaqz commented Jun 1, 2022

Yes that's a good point.

Do you mean all interface components would define a type, and that would be used for dispatch?

So a package implementation declaration might look like:

implements(::Type{<:SomeInterface}, ::Type{<:MyObj}) = true
implements(::Type{<:AnotherInterface}, ::Type{<:MyObj}) = true

We could even use inheritance to define the tree:

abstract type AllTheInterfaces end
abstract type SomeInterface <: AllTheInterfaces end
abstract type AnotherInterface <: AllTheInterfaces end

Then a package could just define:

implements(::Type{<:AllTheInterfaces}, ::Type{<:MyObj}) = true

Or specifically choose just:

implements(::Type{<:SomeInterface}, ::Type{<:MyObj}) = true

That should work for testing and docs generation already - just call implements for the list of interface components in the package and run tests/generate docs if true is returned.

Edit: this would take care of the boilerplant declarations:

@implements MyObj TheInterfaceModule [SomeInterface, AnotherInterface]

@rafaqz
Copy link

rafaqz commented Jun 1, 2022

Another (maybe too far out?) idea is to construct an object in the definition:

@implements MyObj(some, data) TheInterfaceModule [SomeInterface, AnotherInterface]

And run the tests during precompile, so you would also get precompilation for free. Or, add another interface method that retrieves test object/s for the type. That would give access to test objects for other packages to use as well.

@lorenzoh
Copy link
Owner Author

lorenzoh commented Jun 2, 2022

As you suggest, I think a type for an interface is a good idea and dispatching on it in implements as well.

I would suggest, though, to make an interface a concrete type (struct) with fields to configure the interface, for example to say whether an optional part of the interface should be implemented. For example, for the iterator interface:

Base.@kwdef struct IteratorInterface <: Interface
    length = false  # whether the type has to implement the `length` interface (which is optional)
    size = false
    eltypeknown = nothing  # if `true/false` require a specific return value of `IteratorEltype`  
    # more options here, but you get the point
end

IteratorInterface()  # basic, no optional methods
IteratorInterface(size = true)  # `size(iter)` must be implemented
IteratorInterface(eltypeknown = true)

Then we check the interface for a type using implements, which internally converts the interface definition to an Invariant:

implements(interface::Interface, x) = check(invariant(interface), x) 
# each interface implements an `invariant` method to convert to a checkable invariant type

Then users can check interfaces without touching any invariants themselves and we can even compose interfaces logically, e.g.

implements(IteratorInterface() & ArrayInterface(), [1, 2, 3])  # must satisfy both interfaces

All of this would be a layer on top of the invariants. The most important part here is not that you have some way to check an interface (it is straightforward to write a function to do this on case-by-case basis), but that you can define and compose interfaces declaratively and get rich, precise, contextual error messages.


Example interfaces with optional parts that would benefit from this approach:

@rafaqz
Copy link

rafaqz commented Jun 2, 2022

The most important part here is not that you have some way to check an interface (it is straightforward to write a function to do this on case-by-case basis), but that you can define and compose interfaces declaratively and get rich, precise, contextual error messages.

I totally agree about the error messages and composability. For my use cases an equally important thing is that we can use the interface in multiple places:

  • tests
  • documentation
  • precompilation
  • traits other packages can check are implemented for a type

The last option is the one we will need to be careful about with your struct approach: this needs to be
known at compile time with constant propagation. Interface traits as a use-case was one reason I was thinking about using the abstract type hierarchy. It makes dispatch very easy.

Maybe this should all go in another package: Interfaces.jl that builds on top of Invariants.jl so as not to muddy the concepts.

It could have a sub-package BaseInterfaces.jl to keep Interfaces.jl a lean dependency.

@lorenzoh
Copy link
Owner Author

lorenzoh commented Jun 2, 2022

The last option is the one we will need to be careful about with your struct approach: this needs to be
known at compile time with constant propagation. Interface traits as a use-case was one reason I was thinking about using the abstract type hierarchy. It makes dispatch very easy.

Hadn't thought of that

Maybe this should all go in another package: Interfaces.jl that builds on top of Invariants.jl so as not to muddy the concepts.

As you say, interfaces and invariants as discussed here have some overlap but are not the same concepts, so that seems like a good idea. Right now I'm still experimenting here and there may be some API changes before 0.1.0 but I plan to get there soon because I want to get FluxML/FastAI.jl#230 out. Thinking more of the API for interface definition and checks will help inform that.

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

No branches or pull requests

2 participants