Merge pull request #335 from nfdi4plants/oa_equality

Improve OntologyAnnotation equality

.config/dotnet-tools.json

@@ -3,7 +3,7 @@
   "isRoot": true,
   "tools": {
     "fable": {
-      "version": "4.15.0",
+      "version": "4.16.0",
       "commands": [
         "fable"
       ]

ARCtrl.sln

@@ -15,6 +15,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution
 		pyproject.toml = pyproject.toml
 		README.md = README.md
 		RELEASE_NOTES.md = RELEASE_NOTES.md
+		setup.cmd = setup.cmd
 	EndProjectSection
 EndProject
 Project("{6EC3EE1D-3C4E-46DD-8F32-0CC8E7565705}") = "ARCtrl.CWL", "src\CWL\ARCtrl.CWL.fsproj", "{1CD34A01-D19A-441F-8F3D-6EF69D9DDA8D}"

README.md

@@ -52,17 +52,20 @@ Currently we provide some documentation in form of markdown files in the `/docs`
 
 #### Local Setup
 
+On windows you can use the `setup.cmd` to run the following steps automatically!
+
 1. Setup dotnet tools
 
    `dotnet tool restore`
 
+
 2. Install NPM dependencies
 
-   `npm install`
+    `npm install`
 
 3. Setup python environment
 
-   `py -m venv .venv`
+    `py -m venv .venv`
 
 4. Install [Poetry](https://python-poetry.org/) and dependencies
 

setup.cmd

@@ -0,0 +1,18 @@
+@echo off
+TITLE ARCtrl Setup
+
+ECHO Restore .NET tools
+CALL dotnet tool restore
+
+ECHO Install JavaScript Dependencies
+REM npm is a batch file itself. Must use with "CALL" otherwise will exit afterwards.
+CALL npm i
+
+ECHO Setup Python Virtual Environment
+CALL py -m venv .venv
+
+ECHO Install Python Dependencies
+CALL .\.venv\Scripts\python.exe -m pip install -U pip setuptools
+CALL .\.venv\Scripts\python.exe -m pip install poetry
+CALL .\.venv\Scripts\python.exe -m poetry install --no-root
+ECHO DONE!

src/Core/Helper/HashCodes.fs

@@ -1,16 +1,4 @@
 module ARCtrl.Helper.HashCodes
-
-open Fable.Core
-
-// This is necessary until https://github.com/fable-compiler/Fable/issues/3778 is resolved
-
-#if FABLE_COMPILER_PYTHON
-[<Emit("hasattr($0,\"__hash__\")")>]
-let pyHasCustomHash (obj) : bool = nativeOnly
-
-[<Emit("$0.__hash__()")>]
-let pyCustomHash (obj) : int = nativeOnly
-#endif
 
 let mergeHashes (hash1 : int) (hash2 : int) : int =
     0x9e3779b9 + hash2 + (hash1 <<< 6) + (hash1 >>> 2)
@@ -27,14 +15,7 @@ let hashDateTime (dt : System.DateTime) : int =
 
 
 let hash obj =
-    #if FABLE_COMPILER_PYTHON
-        if pyHasCustomHash obj then
-            pyCustomHash obj
-        else
-            obj.GetHashCode()
-    #else
-        obj.GetHashCode()
-    #endif
+    obj.GetHashCode()
 
 let boxHashOption (a: 'a option) : obj =
     if a.IsSome then a.Value.GetHashCode() else (0).GetHashCode()

src/Core/OntologyAnnotation.fs

@@ -89,8 +89,7 @@ type OntologyAnnotation(?name,?tsr,?tan, ?comments) =
     ///
     /// `asOntobeePurlUrl`: option to return term accession in Ontobee purl-url format (`http://purl.obolibrary.org/obo/MS_1000121`)
     /// </summary>
-    ///// TODO: Not sure if still needed ~Kevin F. 2024.03.20
-    static member toString (oa : OntologyAnnotation, ?asOntobeePurlUrlIfShort: bool) =
+    static member toStringObject (oa : OntologyAnnotation, ?asOntobeePurlUrlIfShort: bool) =
         let asOntobeePurlUrlIfShort = Option.defaultValue false asOntobeePurlUrlIfShort
         {|
             TermName = oa.Name |> Option.defaultValue ""
@@ -131,31 +130,32 @@ type OntologyAnnotation(?name,?tsr,?tan, ?comments) =
         sb.Append("}") |> ignore
         sb.ToString()
 
-    override this.Equals other =
-        match other with
-        | :? OntologyAnnotation as oa -> (this :> System.IEquatable<_>).Equals oa
-        | :? string as s ->           
-            this.NameText = s
-            || 
-            this.TermAccessionShort = s
-            ||
-            this.TermAccessionOntobeeUrl = s
-        | _ -> false
-
-    override this.GetHashCode () = (this.NameText+this.TermAccessionShort).GetHashCode()
-
-    interface System.IEquatable<OntologyAnnotation> with
-        member this.Equals other =
-            if this.TermAccessionNumber.IsSome && other.TermAccessionNumber.IsSome then
-                other.TermAccessionShort = this.TermAccessionShort
-                ||
-                other.TermAccessionOntobeeUrl = this.TermAccessionOntobeeUrl
-            elif this.Name.IsSome && other.Name.IsSome then
-                other.NameText = this.NameText
-            elif this.TermAccessionNumber.IsNone && other.TermAccessionNumber.IsNone && this.Name.IsNone && other.Name.IsNone then
-                true
-            else 
-                false
+    override this.GetHashCode() = 
+        [|
+            HashCodes.boxHashOption this.Name
+            match this.TermSourceREF, this.TANInfo with
+            | None, Some taninfo -> // if we get taninfo we assume tsr to be inferrable by taninfo
+                //HashCodes.hash {|tsr = taninfo.IDSpace; tan = taninfo.IDSpace + ":" + taninfo.LocalID|}
+                HashCodes.boxHashArray [|taninfo.IDSpace; taninfo.IDSpace + ":" + taninfo.LocalID|]
+            | Some tsr, Some taninfo -> // if we get taninfo + tsr we do NOT override tsr
+                //HashCodes.hash {|tsr = tsr; tan = taninfo.IDSpace + ":" + taninfo.LocalID|}
+                HashCodes.boxHashArray [|tsr; taninfo.IDSpace + ":" + taninfo.LocalID|]
+            | Some tsr, None ->
+                let tan = this.TermAccessionNumber |> Option.defaultValue ""
+                //HashCodes.hash {|tsr = tsr; tan = tan|}
+                HashCodes.boxHashArray [|tsr; tan|]
+            | None, None ->
+                let tan = this.TermAccessionNumber |> Option.defaultValue ""
+                let tsr = this.TermAccessionNumber |> Option.defaultValue ""
+                //HashCodes.hash {|tsr = tsr; tan = tan|}
+                HashCodes.boxHashArray [|tsr; tan|]
+            //HashCodes.boxHashSeq this.Comments
+        |]
+        |> HashCodes.boxHashArray
+        |> fun x -> x :?> int
+
+    override this.Equals(obj) : bool =
+        HashCodes.hash this = HashCodes.hash obj
 
     member this.Copy() =
         let nextComments = this.Comments |> ResizeArray.map (fun c -> c.Copy())

src/Core/Person.fs

@@ -128,8 +128,6 @@ type Person(?orcid, ?lastName, ?firstName, ?midInitials, ?email, ?phone, ?fax, ?
                 |> not
         ) persons
 
-
-
     member this.Copy() : Person =
         let nextComments = this.Comments |> ResizeArray.map (fun c -> c.Copy())
         let nextRoles = this.Roles |> ResizeArray.map (fun c -> c.Copy())

src/Core/Process/Component.fs

@@ -85,8 +85,8 @@ type Component =
             Component.make None None cType
 
     /// Get ISATab string entries from an ISAJson Component object
-    static member toISAString (c : Component) =
-        let oa = c.ComponentType |> Option.map OntologyAnnotation.toString |> Option.defaultValue {|TermName = ""; TermAccessionNumber = ""; TermSourceREF = ""|}
+    static member toStringObject (c : Component) =
+        let oa = c.ComponentType |> Option.map OntologyAnnotation.toStringObject |> Option.defaultValue {|TermName = ""; TermAccessionNumber = ""; TermSourceREF = ""|}
         c.ComponentName |> Option.defaultValue "", oa
 
     member this.NameText =

src/Core/Process/Factor.fs

@@ -31,8 +31,8 @@ type Factor =
         Factor.make (Option.fromValueWithDefault "" name) (Option.fromValueWithDefault (OntologyAnnotation()) oa) None
 
     /// Get ISATab string entries from an ISAJson Factor object
-    static member toString (factor : Factor) =
-        factor.FactorType |> Option.map OntologyAnnotation.toString |> Option.defaultValue {|TermName = ""; TermAccessionNumber = ""; TermSourceREF = ""|}
+    static member toStringObject (factor : Factor) =
+        factor.FactorType |> Option.map OntologyAnnotation.toStringObject |> Option.defaultValue {|TermName = ""; TermAccessionNumber = ""; TermSourceREF = ""|}
 
     member this.NameText =
         this.Name

src/Core/Process/MaterialAttribute.fs

@@ -28,8 +28,8 @@ type MaterialAttribute =
         MaterialAttribute.make None (Option.fromValueWithDefault (OntologyAnnotation()) oa)
 
     /// Get ISATab string entries from an ISAJson MaterialAttribute object
-    static member toString (ma : MaterialAttribute) =
-        ma.CharacteristicType |> Option.map OntologyAnnotation.toString |> Option.defaultValue {|TermName = ""; TermAccessionNumber = ""; TermSourceREF = ""|}  
+    static member toStringObject (ma : MaterialAttribute) =
+        ma.CharacteristicType |> Option.map OntologyAnnotation.toStringObject |> Option.defaultValue {|TermName = ""; TermAccessionNumber = ""; TermSourceREF = ""|}  
 
     /// Returns the name of the characteristic as string
     member this.NameText =

src/Core/Process/ProtocolParameter.fs

@@ -30,8 +30,8 @@ type ProtocolParameter =
         ProtocolParameter.make None (Option.fromValueWithDefault (OntologyAnnotation()) oa)
 
     /// Get ISATab string entries from an ISAJson ProtocolParameter object (name,source,accession)
-    static member toString (pp : ProtocolParameter) =
-        pp.ParameterName |> Option.map OntologyAnnotation.toString |> Option.defaultValue {|TermName = ""; TermAccessionNumber = ""; TermSourceREF = ""|}       
+    static member toStringObject (pp : ProtocolParameter) =
+        pp.ParameterName |> Option.map OntologyAnnotation.toStringObject |> Option.defaultValue {|TermName = ""; TermAccessionNumber = ""; TermSourceREF = ""|}       
 
     /// Returns the name of the parameter as string
     member this.NameText =

src/Core/Table/CompositeHeader.fs

@@ -526,5 +526,4 @@ type CompositeHeader =
 
     static member freeText(s:string) = FreeText s
 
-#else
 #endif

src/Spreadsheet/Metadata/Assays.fs

@@ -72,8 +72,8 @@ module Assays =
             let processedFileName =
                 if a.Identifier.StartsWith(Identifier.MISSING_IDENTIFIER) then Identifier.removeMissingIdentifier(a.Identifier) else Identifier.Assay.fileNameFromIdentifier(a.Identifier)
             let i = i + 1
-            let mt = Option.defaultValue (OntologyAnnotation()) a.MeasurementType |> fun mt -> OntologyAnnotation.toString(mt,true)
-            let tt = Option.defaultValue (OntologyAnnotation()) a.TechnologyType |> fun tt -> OntologyAnnotation.toString(tt,true)
+            let mt = Option.defaultValue (OntologyAnnotation()) a.MeasurementType |> fun mt -> OntologyAnnotation.toStringObject(mt,true)
+            let tt = Option.defaultValue (OntologyAnnotation()) a.TechnologyType |> fun tt -> OntologyAnnotation.toStringObject(tt,true)
             do matrix.Matrix.Add ((measurementTypeLabel,i),                       mt.TermName)
             do matrix.Matrix.Add ((measurementTypeTermAccessionNumberLabel,i),    mt.TermAccessionNumber)
             do matrix.Matrix.Add ((measurementTypeTermSourceREFLabel,i),          mt.TermSourceREF)

src/Spreadsheet/Metadata/Conversions.fs

@@ -86,7 +86,7 @@ module Component =
         if cs = [] then {|NameAgg = ""; TermNameAgg = "";  TermAccessionNumberAgg = "";  TermSourceREFAgg = ""; |}
         else
             cs
-            |> List.map Component.toISAString
+            |> List.map Component.toStringObject
             |> List.fold (fun (nameAgg,termAgg,tsrAgg,tanAgg) (name,term) ->     
                 if first then 
                     first <- false
@@ -112,7 +112,7 @@ module ProtocolParameter =
         if oas = [] then {|TermNameAgg = ""; TermAccessionNumberAgg = ""; TermSourceREFAgg = ""|}
         else
             oas
-            |> List.map ProtocolParameter.toString
+            |> List.map ProtocolParameter.toStringObject
             |> List.fold (fun (nameAgg,tsrAgg,tanAgg) term -> 
                 if first then 
                     first <- false

src/Spreadsheet/Metadata/Factors.fs

@@ -51,7 +51,7 @@ module Factors =
         factors
         |> List.iteri (fun i f ->
             let i = i + 1
-            let ft = f.FactorType |> Option.defaultValue (OntologyAnnotation()) |> fun f -> OntologyAnnotation.toString(f,true)
+            let ft = f.FactorType |> Option.defaultValue (OntologyAnnotation()) |> fun f -> OntologyAnnotation.toStringObject(f,true)
             do matrix.Matrix.Add ((nameLabel,i),                    (Option.defaultValue "" f.Name))
             do matrix.Matrix.Add ((factorTypeLabel,i),              ft.TermName)
             do matrix.Matrix.Add ((typeTermAccessionNumberLabel,i), ft.TermAccessionNumber)

src/Spreadsheet/Metadata/OntologyAnnotation.fs

@@ -37,7 +37,7 @@ module OntologyAnnotationSection =
         designs
         |> List.iteri (fun i d ->
             let i = i + 1
-            let oa = OntologyAnnotation.toString(d,true)
+            let oa = OntologyAnnotation.toStringObject(d,true)
             do matrix.Matrix.Add ((label,i),                      oa.TermName)
             do matrix.Matrix.Add ((labelTAN,i),   oa.TermAccessionNumber)
             do matrix.Matrix.Add ((labelTSR,i),         oa.TermSourceREF)

src/Spreadsheet/Metadata/Protocols.fs

@@ -84,7 +84,7 @@ module Protocols =
         protocols
         |> List.iteri (fun i p ->
             let i = i + 1
-            let pt = p.ProtocolType |> Option.defaultValue (OntologyAnnotation()) |> fun pt -> OntologyAnnotation.toString(pt,true)
+            let pt = p.ProtocolType |> Option.defaultValue (OntologyAnnotation()) |> fun pt -> OntologyAnnotation.toStringObject(pt,true)
             let pAgg = p.Parameters |> Option.defaultValue [] |> ProtocolParameter.toAggregatedStrings ';' 
             let cAgg = p.Components |> Option.defaultValue [] |> Component.toAggregatedStrings ';' 
 

src/Spreadsheet/Metadata/Publication.fs

@@ -59,7 +59,7 @@ module Publications =
         publications
         |> List.iteri (fun i p ->
             let i = i + 1
-            let s = Option.defaultValue (OntologyAnnotation()) p.Status |> fun s -> OntologyAnnotation.toString (s,true)
+            let s = Option.defaultValue (OntologyAnnotation()) p.Status |> fun s -> OntologyAnnotation.toStringObject (s,true)
             do matrix.Matrix.Add ((pubMedIDLabel,i),                    (Option.defaultValue "" p.PubMedID))
             do matrix.Matrix.Add ((doiLabel,i),                         (Option.defaultValue "" p.DOI))
             do matrix.Matrix.Add ((authorListLabel,i),                  (Option.defaultValue "" p.Authors))

tests/Core/ARCtrl.Core.Tests.fsproj

@@ -8,6 +8,7 @@
   <ItemGroup>
     <Compile Include="Regex.Tests.fs" />
     <Compile Include="Person.Tests.fs" />
+    <Compile Include="OntologyAnnotation.Tests.fs" />
     <Compile Include="CompositeCell.Tests.fs" />
     <Compile Include="CompositeHeader.Tests.fs" />
     <Compile Include="CompositeColumn.Tests.fs" />

tests/Core/ArcJsonConversion.Tests.fs

@@ -141,7 +141,6 @@ let private tests_ArcTableProcess =
             let p = processes.[0]
             Expect.isSome p.ParameterValues "Process should have parameter values"
             Expect.equal p.ParameterValues.Value.Length 1 "Process should have 1 parameter values"
-            Expect.isTrue(p.ParameterValues.Value.[0] = expectedPPV) "Param value does not match"
             Expect.equal p.ParameterValues.Value.[0] expectedPPV "Param value does not match"
             Expect.isSome p.Inputs "Process should have inputs"
             Expect.equal p.Inputs.Value.Length 1 "Process should have 1 input"

tests/Core/DataModel.Tests.fs

@@ -237,68 +237,6 @@ let componentCastingTests =
         )
     ]
 
-let ontologyAnnotationTests = 
-    testList "ontologyAnnotationTests" [
-        let short = "EFO:0000721"
-        let uri = "http://purl.obolibrary.org/obo/EFO_0000721" 
-        let otherParseable = "http://www.ebi.ac.uk/efo/EFO_0000721"
-        let other = "Unparseable"
-        testList "GetHashCode" [
-            testCase "Empty" (fun () ->
-                let oa1 = OntologyAnnotation()
-                let oa2 = OntologyAnnotation()
-                let h1 = oa1.GetHashCode()
-                let h2 = oa2.GetHashCode()
-                Expect.equal h1 h2 "Hashes should be equal"    
-            )
-            testCase "Equal" (fun () ->
-                let oa1 = OntologyAnnotation("MyOntology",tsr = "EFO",tan = uri)
-                let oa2 = OntologyAnnotation("MyOntology",tsr = "EFO",tan = uri)
-                let h1 = oa1.GetHashCode()
-                let h2 = oa2.GetHashCode()
-                Expect.equal h1 h2 "Hashes should be equal"
-            )
-            testCase "Different" (fun () ->
-                let oa1 = OntologyAnnotation("MyOntology",tsr = "EFO",tan = uri)
-                let oa2 = OntologyAnnotation("YourOntology",tsr = "NCBI",tan = "http://purl.obolibrary.org/obo/NCBI_0000123")
-                let h1 = oa1.GetHashCode()
-                let h2 = oa2.GetHashCode()
-                Expect.notEqual h1 h2 "Hashes should not be equal"
-            )
-        ]
-        testList "fromString" [
-
-            testCase "FromShort" (fun () ->
-                let oa = OntologyAnnotation(tan = short)
-                Expect.equal oa.TermAccessionNumber.Value short "TAN incorrect"
-                Expect.equal oa.TermAccessionShort short "short TAN incorrect"
-                Expect.equal oa.TermAccessionOntobeeUrl uri "short TAN incorrect"
-            )
-            testCase "FromUri" (fun () ->          
-                let oa = OntologyAnnotation(tan = uri)
-                Expect.equal oa.TermAccessionNumber.Value uri "TAN incorrect"
-                Expect.equal oa.TermAccessionShort short "short TAN incorrect"
-                Expect.equal oa.TermAccessionOntobeeUrl uri "short TAN incorrect"
-            )
-            testCase "FromOtherParseable" (fun () ->          
-                let oa = OntologyAnnotation(tan = otherParseable)
-                Expect.equal oa.TermAccessionNumber.Value otherParseable "TAN incorrect"
-                Expect.equal oa.TermAccessionShort short "short TAN incorrect"
-                Expect.equal oa.TermAccessionOntobeeUrl uri "short TAN incorrect"
-            )
-            testCase "FromOther" (fun () ->          
-                let oa = OntologyAnnotation(tan = other)
-                Expect.equal oa.TermAccessionNumber.Value other "TAN incorrect"
-            )
-            testCase "FromOtherWithTSR" (fun () ->          
-                let tsr = "ABC"
-                let oa = OntologyAnnotation(tsr = tsr,tan = other)
-                Expect.equal oa.TermAccessionNumber.Value other "TAN incorrect"
-                Expect.equal oa.TermSourceREF.Value tsr "TSR incorrect"
-            )
-        ]
-    ]
-
 let valueTests = 
 
     testList "ValueTests" [
@@ -351,7 +289,6 @@ let valueTests =
 
 let main =
     testList "DataModelTests" [
-        ontologyAnnotationTests
         componentCastingTests
         valueTests
     ]

tests/Core/Main.fs

@@ -4,6 +4,7 @@ open Fable.Pyxpecto
 
 let all = testSequenced <| testList "Core" [
     DataModel.Tests.main
+    OntologyAnnotation.Tests.main
     Regex.Tests.main
     Person.Tests.main
     CompositeHeader.Tests.main