Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase robustness against OntologyAnnotation mutability issues #380

Merged
merged 1 commit into from
Jun 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Core/Table/ArcTableAux.fs
Original file line number Diff line number Diff line change
Expand Up @@ -310,12 +310,12 @@ module Unchecked =
let rowKeys = Array.map snd col |> Set.ofArray
for rowIndex = 0 to rowCount - 1 do
if not <| rowKeys.Contains rowIndex then
addCellAt (columnIndex,rowIndex,defaultCell) values
addCellAt (columnIndex,rowIndex,defaultCell.Copy()) values
// No values existed in this column. Fill with default cells
| None ->
let defaultCell = getEmptyCellForHeader header None
for rowIndex = 0 to rowCount - 1 do
addCellAt (columnIndex,rowIndex,defaultCell) values
addCellAt (columnIndex,rowIndex,defaultCell.Copy()) values


/// Increases the table size to the given new row count and fills the new rows with the last value of the column
Expand Down
7 changes: 7 additions & 0 deletions src/Core/Table/CompositeCell.fs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,13 @@ type CompositeCell =
| Unitized (v,oa) -> $"{v} {oa.NameText}"
| Data d -> $"{d.NameText}"

member this.Copy() =
match this with
| Term oa -> Term (oa.Copy())
| FreeText s -> FreeText s
| Unitized (v,oa) -> Unitized (v, oa.Copy())
| Data d -> Data (d.Copy())

#if FABLE_COMPILER
//[<CompiledName("Term")>]
static member term (oa:OntologyAnnotation) = CompositeCell.Term(oa)
Expand Down
2 changes: 1 addition & 1 deletion src/Json/Table/CellTable.fs
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,5 @@ module CellTable =
let decodeCell (ot : CellTableArray) : Decoder<CompositeCell> =
Decode.object (fun get ->
let i = get.Required.Raw Decode.int
ot.[i]
ot.[i].Copy()
)
23 changes: 20 additions & 3 deletions tests/Core/ArcTable.Tests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2337,9 +2337,26 @@ let private tests_equality = testList "equality" [
]


//let private tests_fillMissing = testList "fillMissing" [
let private tests_fillMissing = testList "fillMissing" [
testCase "OntologyAnnotationCopied" <| fun _ ->
let table = ArcTable.init "My Table"

// ]
table.AddColumn(CompositeHeader.Input IOType.Source, [|for i in 0 .. 4 do CompositeCell.FreeText $"Source {i}"|])

table.AddColumn(CompositeHeader.Component (OntologyAnnotation.create("instrument model", "MS", "MS:424242")))

let cell = table.Values.[(1,0)]

match cell with
| CompositeCell.Term oa ->
oa.Name <- Some "New Name"
| _ -> failwith "Expected a term"

let otherCell = table.Values.[(1,1)]

Expect.equal (table.Values.[(1,1)]) (table.Values.[(1,2)]) "Should be the same cell"
Expect.notEqual cell otherCell "Should not be the same cell"
]

let main =
testList "ArcTable" [
Expand All @@ -2365,5 +2382,5 @@ let main =
tests_IterColumns
tests_GetHashCode
tests_equality
//tests_fillMissing
tests_fillMissing
]
32 changes: 28 additions & 4 deletions tests/Json/ArcTable.Tests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ let private tests_compressedFilled =
None
None

let private tests = testList "extended" [
testList "compressed" [
let private tests_compressedExtended = testList "compressed_extended" [
testCase "rangeColumnSize" <| fun _ ->
// testTable column should be saved as range column, this should make it smaller than the IO column even though it has more cells
let testTable = ArcTable.init("Test")
Expand All @@ -88,8 +87,33 @@ let private tests = testList "extended" [
let encoded = testTable.ToCompressedJsonString()
let decoded = ArcTable.fromCompressedJsonString encoded
Expect.arcTableEqual decoded testTable "decompressed table should be equal to original table"
testCase "SeparateOntologyAnnotations" <| fun _ ->
let table = ArcTable.init "My Table"

table.AddColumn(
CompositeHeader.Component (OntologyAnnotation.create("instrument model", "MS", "MS:424242")),
[|for i in 0 .. 2 do CompositeCell.createTermFromString $"Term"|]
)

let encoded = table.ToCompressedJsonString()
let decoded = ArcTable.fromCompressedJsonString encoded

Expect.equal (table.Values.[(0,1)]) (decoded.Values.[(0,1)]) "Should be the same cell"

let cell = decoded.Values.[(0,0)]

match cell with
| CompositeCell.Term oa ->
oa.Name <- Some "New Name"
| _ -> failwith "Expected a term"

let otherCell = decoded.Values.[(0,1)]

Expect.equal (table.Values.[(0,1)]) (decoded.Values.[(0,1)]) "Should still be the same cell"
Expect.equal (decoded.Values.[(0,1)]) (decoded.Values.[(0,2)]) "Should also be the same cell"
Expect.notEqual cell otherCell "Should not be the same cell"
]
]


let private tests_dataColumns =
testList "dataColumns" [
Expand Down Expand Up @@ -125,7 +149,7 @@ let private tests_commentColumns =


let main = testList "ArcTable" [
tests
tests_compressedExtended
tests_core
tests_coreEmpty
tests_compressedEmpty
Expand Down
Loading