Ro crate data model #426
Ro crate data model #426
Conversation
kMutagene
force-pushed
the
ro-crate-data-model
branch
from
3553745 to
192812d
Compare
|
I think this is ready for a first round of review @HLWeil. While there is still a lot to do, i would like to prevent this PR from becoming a long-running feature branch. When the base profile type implementation is fine, i would like to get it merged soon so that small additions and improvements to that API can be done incrementally in smaller workpackages/PRs. |
| @@ -59,6 +59,33 @@ module RunTests = | |||
| |> Seq.iter dotnetRun | |||
| } | |||
|
|
|||
| let runTestProject = BuildTask.createFn "runTestProject" [clean; build] (fun config -> | |||
| open DynamicObj | ||
|
|
||
| /// Base interface implemented by all explicitly known objects in our ROCrate profiles. | ||
| type IROCrateObject = |
There was a problem hiding this comment.
something i'd count under "add later in smaller PRs"
| let mutable _schemaType = schemaType | ||
| let mutable _additionalType = additionalType | ||
|
|
||
| member this.Id |
There was a problem hiding this comment.
Could you explain the decision to duplicate the properties exclusively on the type and in the interface? Is it because of ease of access?
Also, is it by design that the members are gettable and settable on the exclusive implementation but only gettable on the implementation of the interface?
There was a problem hiding this comment.
Could you explain the decision to duplicate the properties exclusively on the type and in the interface? Is it because of ease of access
type A =
abstract member B: string
type C() =
interface A with
member this.B = "lol nah"
let lol = C()
lol.B // <- does not work
(lol :> A).B // <- must cast to access member BThere was a problem hiding this comment.
Also, is it by design that the members are gettable and settable on the exclusive implementation but only gettable on the implementation of the interface?
I think this is oversight on my part, forgetting updating the interface after making the members mutable. However, i am not so sure we even need the interface. I'd keep it for now to see if it is needed for our implementation purposes as first expected, and remove it if it is not needed
| @@ -2,5 +2,4 @@ | |||
|
|
|||
| set PYTHONIOENCODING=utf-8 | |||
| dotnet tool restore | |||
| cls | |||
src/ROCrate/ISAProfile/Dataset.fs
Outdated
| inherit ROCrateObject(id = id, schemaType = "schema.org/Dataset", ?additionalType = additionalType) | ||
|
|
||
| //interface implementations | ||
| interface IROCrateObject with |
There was a problem hiding this comment.
Why the explicit implementation of the interface again here?
There was a problem hiding this comment.
You are right, this is unnecessary as it is already implemented by the superclass ROCrateObject
src/ROCrate/ISAProfile/Dataset.fs
Outdated
| //interface implementations | ||
| interface IROCrateObject with | ||
| member this.Id with get () = this.Id | ||
| member this.SchemaType with get (): string = this.SchemaType |
There was a problem hiding this comment.
No property retrieval helpers yet?
There was a problem hiding this comment.
Same as before, filed under "let's add that in another PR"
| ) as this = | ||
| inherit Dataset(id, "Investigation") | ||
| do | ||
| // Properties from CreativeWork |
There was a problem hiding this comment.
? Dataset is a child of CreativeWork
There was a problem hiding this comment.
just ignore them comments, leftovers
| /// | ||
| [<AttachMembers>] | ||
| type Dataset (id: string, ?additionalType: string) = | ||
| inherit ROCrateObject(id = id, schemaType = "schema.org/Dataset", ?additionalType = additionalType) |
There was a problem hiding this comment.
Add type CreativeWork and let Dataset inherit it?
There was a problem hiding this comment.
Or not because of the different subtypes of CreativeWork needing different properties of it?
There was a problem hiding this comment.
I do not think we need an explicit CreativeWork type, as we never use it anywhere else (meaining it can be realized as a generic ROCrateObject for all intents and purposes ). Dataset in contrast is parent of I/S/A and there might be an advantage of being able to return all of them as e.g. a Dataset list
| @@ -0,0 +1,38 @@ | |||
| module Tests.Common | |||
There was a problem hiding this comment.
Fine for now, but when everything is settled we should move it to TestingUtils project
…mplementation from Dataset
This PR aims to provide the base classes to implement direct data models for the following ARC RO-Crate profiles:
- cwl- datamapNote that this currently only implements the ISA profile as that is the only "stable" profile yet.
The data model is currently implementing a common
IROCrateObjectinterface with access to@id,@typeand an optionaladditionalType.Furthermore, all classes inherit from DynamicObj, which is transpilable as of CSBiology/DynamicObj#23, meaning we enable the general addition of any property to these classes.
The current design aims for the least static typing possible, leaving casting up to the caller. This neatly circumvents possible complicated type modelling e.g. for properties that can have multiple types joined via
OR.Parsing, writing, or any interop/conversion with the existing data models will be subject of other PRs
Further additions:
runTestProjectbuild target, that runs a single test project in all 3 languages. Use it like this:build.cmd runtestProject <name>, e.g..\build.cmd runTestProject ROCrate