Breaking change - implement cross module interfaces (v2)

[tk3369]

This PR has the code from #41 and additional documentation updates. It fixes #39.

[codecov[bot]]

Codecov Report

Merging #42 into master will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master       #42   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            7         7           
  Lines          208       209    +1     
=========================================
+ Hits           208       209    +1     

Impacted Files	Coverage Δ
src/BinaryTraits.jl	100.00% <ø> (ø)
src/types.jl	100.00% <ø> (ø)
src/assignment.jl	100.00% <100.00%> (ø)
src/interface.jl	100.00% <100.00%> (ø)
src/trait.jl	100.00% <100.00%> (ø)
src/utils.jl	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update bbc1f2c...a8943a8. Read the comment docs.

[tk3369]

As I tried to verify documentation examples, I realized that the abstract array examples no longer works... It does not work because it's trying to assign a thirdparty type Array{Int64,1} with a local trait HasDimension.

I want to write up our use cases more clearly, so we have the followings:

Case	My Module	Other Module	Notes
1	Def, Impl		Defining and implementing my own traits
2	Impl	Def	Implementing a trait defined by a 3rd party module
3	Def	Impl(*)	Defining a trait that a 3rd party module implicitly implemented

Case 3 is a bit special in that we are defining a trait for something that we don't own. So, how can it be useful?

I think it can be useful when the 3rd party package mandates certain interface but it does not adopt BinaryTraits. I want to ensure that my code is correct and therefore I would define my own traits that are mapped to the expected interface.

@KlausC please let me know what you think / opinion, thanks!

[KlausC]

I realized that the abstract array examples no longer works

Could you define test cases, which I can verify. I am not sure I can reproduce the bugs you mention.

[KlausC]

please let me know what you think / opinion, thanks!

We have those 3 dictionaries __binarytraits_*_map. It would be helpful to write up, in which module they are located for all use cases (for each macro, depending on the dictionary key, where do we look for the dictionary?) Actually I did not understand, why they are not in a central module (that is BinarayTraits). The distribution to different modules complicates the software a lot, I fear.

[tk3369]

Here's the example: https://github.com/tk3369/BinaryTraits.jl/blob/bbc1f2c0573572948f78843723a6c3dced5b1a63/examples/abstract_array.jl#L34-L36

On master:

using BinaryTraits
import Base: size
@trait Dimension prefix Has,No
@implement HasDimension by size()::Tuple
const Array1DInt = Array{Int,1};
@assign Array1DInt with Dimension
@check(Array1DInt) # ✅ Array{Int64,1} has implemented:
                   # 1. DimensionTrait: HasDimension ⇢ size(::<Type>)::Tuple

On this branch:

using BinaryTraits
import Base: size
@trait Dimension prefix Has,No
@implement HasDimension by size()::Tuple
const Array1DInt = Array{Int,1};
@assign Array1DInt with HasDimension
check(Array1DInt)  # ✅ Array{Int64,1} has no interface contract requirements.

[tk3369]

We have those 3 dictionaries __binarytraits_*_map. It would be helpful to write up, in which module they are located for all use cases (for each macro, depending on the dictionary key, where do we look for the dictionary?)

There are 3 dictionaries. We can document this more formally later but here is a quick summary:

1. __binarytraits_traits_map - maps from a data type to a set of can-traits. For example, Duck => Set([CanFly,CanSwim]). It is updated during @assign.

2. __binarytraits_interface_map - maps from a can-trait to a set of contracts. For example, CanFly => Set([c1, c2, c3]) where c1/c2/c3 are contract objects. It is updated during @implement.

3. __binarytraits_composite_trait_map - maps a can-trait to a set of can-traits. For example, CanFlySwim => Set(DataType[CanFly, CanSwim]). It is updated during @trait, but only when multiple underlying traits are mentioned.

In the above use cases table, we are pretty much saying that a trait may be defined and implemented in any module. When I say defined, I mean the use of @trait and @implement which establish trait types and required contracts. When I say implemented, I mean the use of @assign.

Of course, as you mentioned above, the complexity kicks in because we have 3 dictionaries in every module that uses BinaryTraits. The logical choice is to update the respective dictionary based upon the key of the dictionary entry. But that breaks down for case 3 - when you define a trait and interface but assign it to a data type that you do not own.

So, I'd like to propose the following:

Case	My Module (A)	Other Module (B)	Which dictionary?
1	Def, Impl		traits map (A) and interface map (A)
2	Impl	Def	traits map (A) and interface map (B)
3	Def	Impl	traits map (A*) and interface map (A)

Notes: (*) In this case, we are assigning a data type defined in Other Module to the __binarytraits_traits_map dictionary in My Module. This is a little counter-intuitive but it is the only way to go because we cannot define a new field in a foreign module.

If we go down this path, then the check function needs to be a little smarter. To locate the contracts, it can check both places - either in the current module where check function is invoked or in the module where the data type is defined.

So far, I have been ignoring composite traits. I believe it should work like traits map i.e. it should live in the __binarytraits_composite_trait_map dictionary of the module where the composite trait is defined.

I am literally "thinking out loud" here :-) Please let me know if it works.

[tk3369]

Actually I did not understand, why they are not in a central module (that is BinaryTraits). The distribution to different modules complicates the software a lot, I fear.

We used to have these dictionaries centralized but it came with a major problem. When we declare a new trait and assign a data type with that trait, we expect that the traits map to be updated. However, because this code is evaluated during pre-compilation phase (because it's not in a function), the trait map is literally wiped out.

I think it happens because we cannot really store a "dynamic" global state in a foreign module. It actually makes sense because if multiple modules are using BinaryTraits, then BinaryTraits cannot be precompiled in such a way that it knows which modules will be using it.

I just replicated the problem again with v0.1.0 if you are interested. You can take a look at https://github.com/tk3369/BinaryTraitsTestX

[tk3369]

@KlausC To get this going, I'm going to merge this PR to master first. The last case above (case 3) can be fixed with another follow-up PR. Let me know if you'd like to do that.

I will try to spend some time this upcoming weekend on this project...