rafaqz / Interfaces.jl

Macros to define and implement interfaces, to ensure they are checked and correct.
MIT License
72 stars 4 forks source link

Multiple arguments in interface #16

Closed gdalle closed 8 months ago

gdalle commented 9 months ago

Fix #14.

codecov-commenter commented 9 months ago

Codecov Report

Merging #16 (282ac27) into main (c75c5d6) will increase coverage by 6.63%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #16      +/-   ##
==========================================
+ Coverage   75.45%   82.08%   +6.63%     
==========================================
  Files           4        5       +1     
  Lines         110      134      +24     
==========================================
+ Hits           83      110      +27     
+ Misses         27       24       -3     
Files Coverage Δ
src/Interfaces.jl 100.00% <ø> (ø)
src/arguments.jl 100.00% <100.00%> (ø)
src/test.jl 81.72% <100.00%> (+3.94%) :arrow_up:

... and 1 file with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

gdalle commented 9 months ago

@rafaqz this is ready, knowing that once the syntax stabilizes I'd like to significantly improve both examples in the docs. I think we can merge this first though.

Only doubt I have is on the additional type check for the (first field of) the test objects. Since interfaces are declarative anyway, I feel like this adds a constraint which does not need to exist. Perhaps we can ask that one of the fields subtypes the interface type, cause that is presumably always satisfied?

rafaqz commented 9 months ago

But if we dont check the type what proof do we have that the test object is the same as the type we are declaring implements the interface?

Or does that check happen elsewhere?

gdalle commented 9 months ago

But if we dont check the type what proof do we have that the test object is the same as the type we are declaring implements the interface?

We don't have any proof. But then again, we don't have any proof that the user ran the checks in the first place :shrug:

I think maybe checking that one of the fields is a subtype is a good compromise, not necessarily the first one. The goal being not to police the user but to help them spot an omission.

rafaqz commented 9 months ago

There has to be a little policing... so that passing the tests means something to other users.

I originally had this all locked down so there was no way the traits could work without the correct tests passing. Running tests in precompilation can guarantee this and I think we should remain open to returning to that stratedgy.

Maybe we need a standard argument name for the target object rather than the first, then the tests are easier to read.

(The point is to make interfaces like haskell where an objective compiler tells you what to do and there is no room for avoiding the interface. The loose social nature of Julia interfaces turns out to have a huge and unnecessary organisational overhead)

gdalle commented 9 months ago

But in the group scenario for instance there are several target objects. And I think policing field names is not great

rafaqz commented 9 months ago

I dont mind what we police, but knowing we are actually testing the type declared to implement the interface is important.

We just need a design

gdalle commented 9 months ago

Maybe Arguments{T} where T is the interface-implementing type? And then we check in the background that at least one field of args subtypes T?

gdalle commented 9 months ago

That way we can test at argument construction

rafaqz commented 9 months ago

Although that will make the implementer specify the type in Arguments. But we already have it available in test so we can just check all argument types there anyway?

Im still wondering if maybe we can get the correct argument name from @implements somehow so its totally locked down.

But maybe we dont have to get it perfect straight away.

gdalle commented 9 months ago

How about:

  1. The instances of the interface-implementing type must have a name starting with obj (which allows obj1 and obj2)
  2. There must be at least one such instance in the fields of an Arguments
rafaqz commented 9 months ago

Yes that could work. Lets sit on this for a while and just test that any one argument matches the type in test fir now

gdalle commented 9 months ago

Ok I'll write that. Just a silly question: given a tuple type Tuple{Int,Float64,String}, how would you list the field types? I couldn't figure that one out

rafaqz commented 9 months ago

fieldtypes(T)?

Or just map over the values with

any(map(x -> x isa T, nt(args)))
gdalle commented 9 months ago

Yeah I wanted to deduce it from the type itself but fieldtypes didn't work, just wondered if you had encountered that before. I'll check the values instead

gdalle commented 8 months ago

@rafaqz this should be good to go as a first draft, even if we end up refining it later

rafaqz commented 8 months ago

Ok looks like it just needs a rebase

gdalle commented 8 months ago

done