Orcid

ARCtrl.sln

@@ -55,6 +55,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "playground", "playground",
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "JavaScript", "JavaScript", "{913222CA-261F-49CB-A823-CC7C335F964A}"
 	ProjectSection(SolutionItems) = preProject
+		tests\JavaScript\ISA.Person.js = tests\JavaScript\ISA.Person.js
 		tests\JavaScript\Main.js = tests\JavaScript\Main.js
 	EndProjectSection
 EndProject

src/ISA/ISA.Json/Person.fs

@@ -32,8 +32,9 @@ module Person =
                                 | _ -> "#EmptyPerson"
 
     let rec encoder (options : ConverterOptions) (oa : obj) = 
+        let oa = oa :?> Person |> Person.setCommentFromORCID
         [
-            if options.SetID then "@id", GEncode.string (oa :?> Person |> genID)
+            if options.SetID then "@id", GEncode.string (oa |> genID)
                 else GEncode.tryInclude "@id" GEncode.string (oa |> GEncode.tryGetPropertyValue "ID")
             if options.IncludeType then "@type", GEncode.string "Person"
             GEncode.tryInclude "firstName" GEncode.string (oa |> GEncode.tryGetPropertyValue "FirstName")
@@ -54,6 +55,7 @@ module Person =
         Decode.object (fun get ->
             {
                 ID = get.Optional.Field "@id" GDecode.uri
+                ORCID = None
                 FirstName = get.Optional.Field "firstName" Decode.string
                 LastName = get.Optional.Field "lastName" Decode.string
                 MidInitials = get.Optional.Field "midInitials" Decode.string
@@ -65,6 +67,7 @@ module Person =
                 Roles = get.Optional.Field "roles" (Decode.array (OntologyAnnotation.decoder options))
                 Comments = get.Optional.Field "comments" (Decode.array (Comment.decoder options))
             }
+            |> Person.setOrcidFromComments
 
         )
 

src/ISA/ISA.Spreadsheet/InvestigationFile/Contacts.fs

@@ -25,6 +25,7 @@ module Contacts =
         let roles = OntologyAnnotation.fromAggregatedStrings ';' role rolesTermSourceREF rolesTermAccessionNumber
         Person.make 
             None 
+            None
             (Option.fromValueWithDefault "" lastName   ) 
             (Option.fromValueWithDefault "" firstName  )
             (Option.fromValueWithDefault "" midInitials) 
@@ -35,6 +36,7 @@ module Contacts =
             (Option.fromValueWithDefault "" affiliation) 
             (Option.fromValueWithDefault [||] roles    )
             (Option.fromValueWithDefault [||] comments )
+        |> Person.setOrcidFromComments
 
     let fromSparseTable (matrix : SparseTable) =
         if matrix.ColumnCount = 0 && matrix.CommentKeys.Length <> 0 then
@@ -67,6 +69,7 @@ module Contacts =
         let matrix = SparseTable.Create (keys = labels,length=persons.Length + 1)
         let mutable commentKeys = []
         persons
+        |> List.map Person.setCommentFromORCID
         |> List.iteri (fun i p ->
             let i = i + 1
             let rAgg = Option.defaultValue [||] p.Roles |> OntologyAnnotation.toAggregatedStrings ';'

src/ISA/ISA.Spreadsheet/SparseTable.fs

@@ -48,7 +48,8 @@ module SparseRow =
     let writeToSheet (rowI : int) (row : SparseRow) (sheet : FsWorksheet) =
         let fsRow = sheet.Row(rowI)
         row
-        |> Seq.iter (fun (colI,v) -> fsRow.[colI + 1].SetValueAs v)
+        |> Seq.iter (fun (colI,v) -> 
+            if v.Trim() <> "" then fsRow.[colI + 1].SetValueAs v)
 
 type SparseTable = 
 

src/ISA/ISA/JsonTypes/Person.fs

@@ -8,6 +8,7 @@ open Fable.Core
 type Person = 
     {   
         ID : URI option
+        ORCID : string option
         LastName : string option
         FirstName : string option
         MidInitials : string option
@@ -20,9 +21,10 @@ type Person =
         Comments : Comment [] option  
     }
 
-    static member make id lastName firstName midInitials email phone fax address affiliation roles comments : Person =
+    static member make id orcid lastName firstName midInitials email phone fax address affiliation roles comments : Person =
         {
             ID = id
+            ORCID = orcid
             LastName = lastName
             FirstName = firstName
             MidInitials = midInitials
@@ -35,8 +37,8 @@ type Person =
             Comments = comments
         }
 
-    static member create (?Id,?LastName,?FirstName,?MidInitials,?Email,?Phone,?Fax,?Address,?Affiliation,?Roles,?Comments) : Person =
-        Person.make Id LastName FirstName MidInitials Email Phone Fax Address Affiliation Roles Comments
+    static member create (?Id,?ORCID,?LastName,?FirstName,?MidInitials,?Email,?Phone,?Fax,?Address,?Affiliation,?Roles,?Comments) : Person =
+        Person.make Id ORCID LastName FirstName MidInitials Email Phone Fax Address Affiliation Roles Comments
 
     static member empty =
         Person.create ()
@@ -129,11 +131,45 @@ type Person =
         { person with
             Comments = Some comments }
 
+    static member orcidKey = "ORCID"
+
+    static member setOrcidFromComments (person : Person) =
+        let isOrcidComment (c : Comment) = 
+            c.Name.IsSome && (c.Name.Value.ToUpper().EndsWith(Person.orcidKey))
+        let orcid,comments = 
+            person.Comments
+            |> Option.map (fun comments ->
+                let orcid = 
+                    comments
+                    |> Array.tryPick (fun c -> if isOrcidComment c then c.Value else None)
+                let comments = 
+                    comments
+                    |> Array.filter (isOrcidComment >> not)
+                    |> Option.fromValueWithDefault [||]
+                (orcid, comments)
+            )
+            |> Option.defaultValue (None, person.Comments)
+        {person with ORCID = orcid; Comments = comments}
+
+    static member setCommentFromORCID (person : Person) =
+        let comments = 
+            match person.ORCID, person.Comments with
+            | Some orcid, Some comments -> 
+                let comment = Comment.create (Name = Person.orcidKey, Value = orcid)
+                Array.append comments [|comment|]
+                |> Some
+            | Some orcid, None -> 
+                [|Comment.create (Name = Person.orcidKey, Value = orcid)|]
+                |> Some
+            | None, comments -> comments
+        {person with Comments = comments}
+
     member this.Copy() : Person =
         let nextComments = this.Comments |> Option.map (Array.map (fun c -> c.Copy()))
         let nextRoles = this.Roles |> Option.map (Array.map (fun c -> c.Copy()))
         Person.make 
             this.ID 
+            this.ORCID
             this.LastName 
             this.FirstName 
             this.MidInitials 

tests/ISA/ISA.Json.Tests/Json.Tests.fs

@@ -881,6 +881,48 @@ let testPersonFile =
 
             mySequenceEqual actual expected "Written person file does not match read person file"
         )
+
+
+        testCase "WithORCID ReaderCorrectness" (fun () -> 
+
+            let p = Person.fromString TestObjects.Person.personWithORCID
+            Expect.isNone p.Comments "Comments should be None"
+            Expect.isSome p.ORCID "ORCID should be Some"
+            Expect.equal p.ORCID.Value "0000-0002-1825-0097" "ORCID not as expected"
+
+        )
+
+        testAsync "WithORCID WriterSchemaCorrectness" {
+
+            let a = Person.fromString TestObjects.Person.personWithORCID
+
+            let s = Person.toString a
+
+            let! validation = Validation.validatePerson s
+
+            Expect.isTrue validation.Success $"Person did not match schema: {validation.GetErrors()}"
+        }
+
+        testCase "WithORCID OutputMatchesInput" (fun () ->
+
+            let o = 
+                Person.fromString TestObjects.Person.personWithORCID
+                |> Person.toString
+
+            let expected = 
+                TestObjects.Person.personWithORCID
+                |> Utils.extractWords
+                |> Array.countBy id
+                |> Array.sortBy fst
+
+            let actual = 
+                o
+                |> Utils.extractWords
+                |> Array.countBy id
+                |> Array.sortBy fst
+
+            mySequenceEqual actual expected "Written person file does not match read person file"
+        )
     ]
 
 let testPersonFileLD =
@@ -1265,6 +1307,7 @@ let testInvestigationFile =
             let person =
                 Person.make
                     (Some "Persons/LukasWeil")
+                    None
                     (Some "Weil")
                     (Some "Lukas")
                     (Some "H")
@@ -1687,6 +1730,7 @@ let testInvestigationFileLD =
             let person =
                 Person.make
                     (Some "Persons/LukasWeil")
+                    None
                     (Some "Weil")
                     (Some "Lukas")
                     (Some "H")

tests/ISA/ISA.Json.Tests/TestObjects/Person.fs

@@ -26,6 +26,20 @@ let person =
 }
     """
 
+let personWithORCID = 
+    """
+    {
+  "firstName": "Juan",
+  "lastName": "Castrillo",
+  "comments": [
+    {
+      "value": "0000-0002-1825-0097",
+      "name": "ORCID"
+    }
+  ]
+  }
+    """
+
 let personLD = 
     """
     {

tests/ISA/ISA.Spreadsheet.Tests/AssayFileTests.fs

@@ -29,6 +29,18 @@ let testMetaDataFunctions =
 
         )
 
+        testCase "ReaderReadsORCID" (fun () -> 
+
+            let assay = ArcAssay.fromMetadataSheet TestObjects.Assay.assayMetadata 
+            Expect.equal assay.Performers.Length 3 "Assay should have 3 performers"
+            Expect.isSome assay.Performers.[0].ORCID "ORCID should be set"
+            Expect.equal assay.Performers.[0].ORCID.Value "0000-0002-1825-0097" "ORCID not read correctly"
+            Expect.isNone assay.Performers.[1].ORCID "ORCID should not be set"
+            Expect.isSome assay.Performers.[2].ORCID "ORCID should be set"
+            Expect.equal assay.Performers.[2].ORCID.Value "0000-0002-1825-0098" "ORCID not read correctly"
+
+        )
+
         testCase "ReaderSuccessObsoleteSheetName" (fun () -> 
 
             let readingSuccess = 
@@ -56,6 +68,16 @@ let testMetaDataFunctions =
             Expect.isOk writingSuccess (Result.getMessage writingSuccess)
         )
 
+        testCase "WriterCreatesNoEmptyCells" (fun () ->
+
+            let o = 
+                ArcAssay.fromMetadataSheet TestObjects.Assay.assayMetadata
+                |> ArcAssay.toMetadataSheet
+
+            o.CellCollection.GetCells()
+            |> Seq.iter (fun c -> Expect.notEqual (c.Value.Trim()) "" $"Cell {c.Address.ToString()} should not contain empty string")  
+        )
+
         testCase "WriterSuccessObsoleteSheetName" (fun () ->
 
             let a = ArcAssay.fromMetadataSheet TestObjects.Assay.assayMetadataObsoleteSheetName

tests/ISA/ISA.Spreadsheet.Tests/TestObjects/Assay.fs

@@ -76,6 +76,10 @@ let assayMetadata =
     row22.[2].Value <- "Sheet1;Sheet2"
     row22.[3].Value <- "Sheet2"
     row22.[4].Value <- "Sheet3"
+    let row23 = ws.Row(23)
+    row23.[1].Value <- "Comment[ORCID]"
+    row23.[2].Value <- "0000-0002-1825-0097"
+    row23.[4].Value <- "0000-0002-1825-0098"
     ws
 
 let assayMetadataDeprecatedKeys = 
@@ -152,6 +156,10 @@ let assayMetadataDeprecatedKeys =
     row22.[2].Value <- "Sheet1;Sheet2"
     row22.[3].Value <- "Sheet2"
     row22.[4].Value <- "Sheet3"
+    let row23 = ws.Row(23)
+    row23.[1].Value <- "Comment[Assay ORCID]"
+    row23.[2].Value <- "0000-0002-1825-0097"
+    row23.[4].Value <- "0000-0002-1825-0098"
     ws
 
 let assayMetadataObsoleteSheetName = 

tests/ISA/ISA.Tests/Person.Tests.fs

@@ -10,6 +10,48 @@ open Expecto
 
 let private createTestPerson() = Person.create(FirstName="Kevin", LastName="Frey",Email="MyAwesomeMail@Awesome.mail")
 
+let private tests_ORCID =
+    testList "ORCID" [
+        testCase "fromComment correctORCID" (fun () ->
+            let comment = Comment.create(Name = Person.orcidKey, Value = "0000-0002-1825-0097")
+            let p = Person.create(FirstName="My", LastName="Dude",Comments =[|comment|])
+            let newP = Person.setOrcidFromComments p
+            Expect.isSome newP.ORCID "ORCID should now be set" 
+            Expect.equal newP.ORCID.Value "0000-0002-1825-0097" "ORCID not taken correctly"
+            Expect.isNone newP.Comments "Comments not set to None"
+        )
+        testCase "fromComment wrongORCID" (fun () ->
+            let comment = Comment.create(Name = "WrongKey", Value = "0000-0002-1825-0097")
+            let p = Person.create(FirstName="My", LastName="Dude",Comments =[|comment|])
+            let newP = Person.setOrcidFromComments p
+            Expect.isNone newP.ORCID "ORCID should not have been taken"
+            Expect.isSome newP.Comments "Comments should still be there"
+            Expect.equal newP.Comments.Value.Length 1 "Comments should still be there"
+        )
+        testCase "fromComment deprecatedORCID" (fun () ->
+            let comment = Comment.create(Name = "Investigation Person Orcid", Value = "0000-0002-1825-0097")
+            let p = Person.create(FirstName="My", LastName="Dude",Comments =[|comment|])
+            let newP = Person.setOrcidFromComments p
+            Expect.isSome newP.ORCID "ORCID should now be set" 
+            Expect.equal newP.ORCID.Value "0000-0002-1825-0097" "ORCID not taken correctly"
+            Expect.isNone newP.Comments "Comments not set to None"
+        )
+        testCase "toComment SomeORCID" (fun () ->
+            let p = Person.create(FirstName="My", LastName="Dude",ORCID = "0000-0002-1825-0097")
+            let newP = Person.setCommentFromORCID p
+            Expect.isSome newP.Comments "Comments should now be set" 
+            Expect.equal newP.Comments.Value.Length 1 "Comments should now be set" 
+            Expect.equal newP.Comments.Value.[0].Name.Value Person.orcidKey "Comments should now be set" 
+            Expect.equal newP.Comments.Value.[0].Value.Value "0000-0002-1825-0097" "Comments should now be set"               
+        ) 
+        testCase "toComment NoneORCID" (fun () ->
+            let p = Person.create(FirstName="My", LastName="Dude")
+            let newP = Person.setCommentFromORCID p
+            Expect.isNone newP.Comments "Comments should not be set" 
+        )
+
+    ]
+
 let private tests_Copy = testList "Copy" [
     testCase "test copy" <| fun _ ->
         // this test is silly in dotnet as record types are immutable. Test this in js native too!
@@ -24,4 +66,5 @@ let private tests_Copy = testList "Copy" [
 let main = 
     testList "Person" [
         tests_Copy
+        tests_ORCID
     ]

tests/JavaScript/ISA.Person.js

@@ -5,8 +5,10 @@ import { Comment$ as Comment } from "./ARCtrl/ISA/ISA/JsonTypes/Comment.js";
 describe('ISA.Person', function () {
     describe('copy', function () {
         it('check mutable', function () {
-            const person = Person.create(void 0, "Frey", "Kevin", void 0, "MyAwesomeMail@Awesome.mail");
+            const person = Person.create(void 0, "0000-0002-1825-0097", "Frey", "Kevin", void 0, "MyAwesomeMail@Awesome.mail");
             const copy = person.Copy()
+            equal(person.ORCID, "0000-0002-1825-0097")
+            equal(copy.ORCID, "0000-0002-1825-0097")
             equal(person.FirstName, "Kevin");
             equal(copy.FirstName, "Kevin");
             person.FirstName = "NotKevin"
@@ -15,7 +17,7 @@ describe('ISA.Person', function () {
         });
         it('check nested mutable', function(){
             const comment = Comment.fromString("TestKey", "TestValue")
-            const person = Person.create(void 0, "Frey", "Kevin", void 0, "MyAwesomeMail@Awesome.mail", void 0, void 0, void 0, void 0, void 0, [comment]);
+            const person = Person.create(void 0, void 0, "Frey", "Kevin", void 0, "MyAwesomeMail@Awesome.mail", void 0, void 0, void 0, void 0, void 0, [comment]);
             const copy = person.Copy()
             equal(person.Comments.length,1)
             deepEqual(person.Comments,copy.Comments, "Should be same")