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.

Sign up for GitHub

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

Jump to bottom

Restore partial functionality of button Validate / Restore Ontology Term #517

Merged
merged 5 commits into from
Oct 7, 2024
Merged

Restore partial functionality of button Validate / Restore Ontology Term #517

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
215a175
Restore partial functionality of button Validate / Restore Ontology Term
Etschbeijer Aug 29, 2024
68b958b
Minor adaptions
Etschbeijer Sep 4, 2024
184237e
Implement requested review changes
Etschbeijer Oct 7, 2024
ddbaab1
Implement review requests
Etschbeijer Oct 7, 2024
4585561
Implement review request
Etschbeijer Oct 7, 2024
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
8 changes: 4 additions & 4 deletions src/Client/OfficeInterop/Functions/BuildingBlockFunctions.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,10 +298,10 @@ let toTermSearchable (buildingBlock:BuildingBlock) =
// get all units from cells
|> Array.map (fun cell -> cell.Unit, cell.Index)
// filter units to unique
|> Array.choose (fun (unitName,rowInd) -> if unitName.IsSome then Some (unitName.Value,rowInd) else None)
|> Array.choose (fun (unitName, rowInd) -> if unitName.IsSome then Some (unitName.Value, rowInd) else None)
|> Array.groupBy fst
// get only units where unit.isSome
|> Array.map (fun (unitTerm,cellInfoArr) ->
|> Array.map (fun (unitTerm, cellInfoArr) ->
let cellRowIndices = cellInfoArr |> Array.map snd |> Array.distinct
// will not contain termAccession
TermSearchable.create unitTerm None true colIndex cellRowIndices
Expand All @@ -314,10 +314,10 @@ let toTermSearchable (buildingBlock:BuildingBlock) =
// get all terms from cells
|> Array.map (fun cell -> cell.Value, cell.Index)
// get only values where value.isSome
|> Array.choose (fun (valueName,rowInd) -> if valueName.IsSome then Some (valueName.Value,rowInd) else None)
|> Array.choose (fun (valueName, rowInd) -> if valueName.IsSome then Some (valueName.Value, rowInd) else None)
|> Array.groupBy fst
// filter terms to unique
|> Array.map (fun (valueName,cellInfoArr) ->
|> Array.map (fun (valueName, cellInfoArr) ->
let cellRowIndices = cellInfoArr |> Array.map snd |> Array.distinct
let tryFindAccession =
buildingBlock.TAN.Value.Cells
Expand Down
174 changes: 143 additions & 31 deletions src/Client/OfficeInterop/OfficeInterop.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,10 @@ module OfficeInteropExtensions =
/// <param name="table"></param>
/// <param name="columnIndex"></param>
/// <param name="context"></param>
let getChosenBuildingBlock (table: Table) (columnIndex: float) (context: RequestContext) =
let getBuildingBlockByIndex (table: Table) (columnIndex: float) (context: RequestContext) =

promise {

let selectedRange = context.workbook.getSelectedRange().load(U2.Case2 (ResizeArray[|"columnIndex"|]))

let headerRange = table.getHeaderRowRange()
let _ = headerRange.load(U2.Case2 (ResizeArray [|"columnIndex"; "values"; "columnCount"|])) |> ignore

Expand Down Expand Up @@ -104,6 +102,14 @@ module OfficeInteropExtensions =
|> Seq.map (snd >> Seq.map snd >> Seq.toArray)
|> Seq.toArray

/// <summary>
/// Add a new column at the given index
/// </summary>
/// <param name="index"></param>
/// <param name="excelTable"></param>
/// <param name="name"></param>
/// <param name="rowCount"></param>
/// <param name="value"></param>
static member addColumn (index:float) (excelTable:Table) name rowCount value =
let col = createMatrixForTables 1 rowCount value

Expand Down Expand Up @@ -1136,14 +1142,13 @@ let replaceOutputColumn (excelTableName:string) (existingOutputColumn: BuildingB
()
)

let! fit = autoFitTableByTable excelTable context
let! _ = autoFitTableByTable excelTable context

let warningMsg = $"Found existing output column \"{existingOutputColumn.MainColumn.Header.SwateColumnHeader}\". Changed output column to \"{newOutputcolumn.ColumnHeader.toAnnotationTableHeader()}\"."

let msg = InteropLogging.Msg.create InteropLogging.Warning warningMsg

let loggingList = [
msg
]
let loggingList = [ msg ]

return loggingList
}
Expand Down Expand Up @@ -1183,9 +1188,7 @@ let updateInputColumn (excelTable:Table) (arcTable:ArcTable) (newBB:CompositeCol

let msg = InteropLogging.Msg.create InteropLogging.Warning warningMsg

let loggingList = [
msg
]
let loggingList = [ msg ]

loggingList
else
Expand Down Expand Up @@ -1215,9 +1218,7 @@ let addInputColumn (excelTable:Table) (arcTable:ArcTable) (newBB:CompositeColumn

let msg = InteropLogging.Msg.create InteropLogging.Info $"Added new input column: {newBB.Header}"

let loggingList = [
msg
]
let loggingList = [ msg ]

loggingList

Expand Down Expand Up @@ -1255,9 +1256,7 @@ let updateOutputColumn (excelTable:Table) (arcTable:ArcTable) (newBB:CompositeCo

let msg = InteropLogging.Msg.create InteropLogging.Warning warningMsg

let loggingList = [
msg
]
let loggingList = [ msg ]

loggingList
else
Expand Down Expand Up @@ -1288,9 +1287,7 @@ let addOutputColumn (excelTable:Table) (arcTable:ArcTable) (newBB:CompositeColum

let msg = InteropLogging.Msg.create InteropLogging.Info $"Added new output column: {newBB.Header}"

let loggingList = [
msg
]
let loggingList = [ msg ]

loggingList

Expand Down Expand Up @@ -1347,13 +1344,15 @@ let addBuildingBlock (excelTable:Table) (arcTable:ArcTable) (newBB:CompositeColu

buildingBlockCells
|> List.iteri(fun i bbCell ->
let mutable newHeader = bbCell.Head
//check and extend header to avoid duplicates
newHeader <- Indexing.extendName (headers |> List.toArray) bbCell.Head
//check and extend header to avoid duplicates
let newHeader = Indexing.extendName (headers |> List.toArray) bbCell.Head
let calIndex =
if targetIndex >= 0 then targetIndex + (float) i
else AppendIndex
let column = ExcelHelper.addColumn calIndex excelTable newHeader rowCount bbCell.Tail.Head
let value =
if bbCell.Tail.IsEmpty then ""
else bbCell.Tail.Head
let column = ExcelHelper.addColumn calIndex excelTable newHeader rowCount value
newHeader::headers |> ignore
column.getRange().format.autofitColumns()

Expand All @@ -1363,9 +1362,7 @@ let addBuildingBlock (excelTable:Table) (arcTable:ArcTable) (newBB:CompositeColu

let msg = InteropLogging.Msg.create InteropLogging.Info $"Added new term column: {newBB.Header}"

let loggingList = [
msg
]
let loggingList = [ msg ]

loggingList

Expand Down Expand Up @@ -1957,12 +1954,128 @@ let validateAnnotationTable () =
[InteropLogging.Msg.create InteropLogging.Warning $"Table is not a valid ARC table / ISA table: {ex.Message}. The column {header} is not valid! It needs further inspection what causes the error.";
])
else
[InteropLogging.Msg.create InteropLogging.Warning $"The annotation table {excelTable.name} is valid"]
[]

return messages
}
)

let fillEmptyBuildingBlocks () =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to autoCompleteFromExistingTerms, add extensive comment in xml style to explain functionality.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Excel.run(fun context ->
promise {

let! msg = validateAnnotationTable ()
let mutable messages = msg

//When msg > 0 there is an error and we can skip the filling of the rows because the columns are not valid
if messages.Length > 0 then return messages
else
let! excelTable = getActiveAnnotationTable context
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must parse table a second time after validateAnnotationTable. Maybe have validateAnnotationTable have return the table if validated successful. This would require a new ouput using Result

let tryString (x: string) =
  if x.StartsWith "y" then
    Result.Ok true
  else
    Result.Error false


match tryString "yes" with
| Result.Ok x -> printfn "Ok: %A" x
| Result.Error x -> printfn "Error: %A" x

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use match case instead of if-else on line 1971

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed


//Arctable enables a fast check for term and unit building blocks
let! arcTable = ArcTable.tryGetFromExcelTable(excelTable, context)

let arcTable =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

already checked

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

if arcTable.IsSome then arcTable.Value
else failwith "No arc table is available"

let columns = arcTable.Columns

let _ = excelTable.columns.load(propertyNames = U2.Case2 (ResizeArray[|"count"; "items"; "name"; "values"; "rowCount"|]))

do! context.sync().``then``(fun _ -> ())

let items = excelTable.columns.items

do! context.sync().``then``(fun _ -> ())

let termAndUnitHeaders = columns |> Array.choose (fun item -> if item.Header.IsTermColumn then Some (item.Header.ToString()) else None)

let columns =
items
|> Array.ofSeq
|> Array.map (fun c ->
c.values
|> Array.ofSeq
|> Array.collect (fun cc ->
cc
|> Array.ofSeq
|> Array.choose (fun r -> if r.IsSome then Some (r.ToString()) else None)
)
)

let body = excelTable.getDataBodyRange()

let _ = body.load(propertyNames = U2.Case2 (ResizeArray[|"values"|]))

do! context.sync().``then``(fun _ -> ())

for i in 0..columns.Length-1 do
let column = columns.[i]
if Array.contains (column.[0].Trim()) termAndUnitHeaders then
let! potBuildingBlock = getBuildingBlockByIndex excelTable i context
let buildingBlock = potBuildingBlock |> Array.ofSeq

// Check whether building block is unit or not
// When it is unit, then delete the property column values only when the unit is empty, independent of the main column
// When it is a term, then delete the property column values when the main column is empty
let mainColumn =
if snd buildingBlock.[1] = "Unit" then
excelTable.columns.items.Item (fst buildingBlock.[1])
else excelTable.columns.items.Item (fst buildingBlock.[0])
let potPropertyColumns =
buildingBlock.[1..]
|> Array.map (fun (index, _) -> index, excelTable.columns.items.Item index)

let propertyColumns =
potPropertyColumns
|> Array.map (fun (index, c) ->
index,
c.values
|> Array.ofSeq
|> Array.collect (fun pc ->
pc
|> Array.ofSeq
|> Array.choose (fun pcv -> if pcv.IsSome then Some (pcv.ToString()) else None)
)
)

let mainColumnHasValues =
mainColumn.values
|> Array.ofSeq
|> Array.collect (fun c ->
c
|> Array.ofSeq
|> Array.choose (fun cc -> if cc.IsSome then Some (cc.ToString()) else None)
|> Array.map (fun cv -> cv, String.IsNullOrEmpty(cv))
)

//Check whether value of property colum is fitting for value of main column and adapt if not
//Delete values of property columns when main column is empty
propertyColumns
|> Array.iter (fun (pIndex, pcv) ->
let values = Array.create (arcTable.RowCount + 1) ""
mainColumnHasValues
|> Array.iteri (fun mainIndex (mc, isNull) ->
//if isNull for main column, then use empty string as value for properties
if not isNull then
values.[mainIndex] <- pcv.[mainIndex]
)

let bodyValues =
values
|> Array.map (box >> Some)
|> Array.map (fun c -> ResizeArray[c])
|> ResizeArray
excelTable.columns.items.[pIndex].values <- bodyValues
)

//do! ExcelHelper.adoptTableFormats(excelTable, context, true)

return [InteropLogging.Msg.create InteropLogging.Warning $"The annotation table {excelTable.name} is valid"]
}
)

// Old stuff, mostly deprecated

let private createColumnBodyValues (insertBB:InsertBuildingBlock) (tableRowCount:int) =
Expand Down Expand Up @@ -1997,7 +2110,7 @@ let private createColumnBodyValues (insertBB:InsertBuildingBlock) (tableRowCount
let tans = createList rowCount (insertBB.Rows |> Array.map (fun tm -> tm.accessionToTAN))
[|termNames; tsrs; tans|]

let addAnnotationBlocksToTable (buildingBlocks:InsertBuildingBlock [], table:Table,context:RequestContext) =
let addAnnotationBlocksToTable (buildingBlocks:InsertBuildingBlock [], table:Table, context:RequestContext) =
promise {

let excelTable = table
Expand All @@ -2009,7 +2122,7 @@ let addAnnotationBlocksToTable (buildingBlocks:InsertBuildingBlock [], table:Tab
/// alreadyExistingBBs -> will be used for logging
let newBuildingBlocks, alreadyExistingBBs =
let newSet = buildingBlocks |> Array.map (fun x -> x.ColumnHeader) |> Set.ofArray
let prevSet = existingBuildingBlocks |> Array.choose (fun x -> x.MainColumn.Header.toBuildingBlockNamePrePrint )|> Set.ofArray
let prevSet = existingBuildingBlocks |> Array.choose (fun x -> x.MainColumn.Header.toBuildingBlockNamePrePrint) |> Set.ofArray
let bbsToAdd = Set.difference newSet prevSet |> Set.toArray
// These building blocks do not exist in table and will be added
let newBuildingBlocks =
Expand Down Expand Up @@ -2394,8 +2507,7 @@ let checkForDeprecation (buildingBlocks:BuildingBlock []) =
let message = InteropLogging.Msg.create InteropLogging.Warning m
msgList <- message::msgList
buildingBlocks
| None ->
buildingBlocks
| None -> buildingBlocks
// Chain all deprecation checks
buildingBlocks
|> deprecated_DataFileName
Expand Down
2 changes: 1 addition & 1 deletion src/Client/SidebarComponents/Navbar.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ let private shortCutIconList model dispatch =
Html.i [prop.className "fa-solid fa-pen"]
],
(fun _ ->
SpreadsheetInterface.ValidateAnnotationTable |> InterfaceMsg |> dispatch
//SpreadsheetInterface.ValidateAnnotationTable |> InterfaceMsg |> dispatch
SpreadsheetInterface.UpdateTermColumns |> InterfaceMsg |> dispatch
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe coin a term for this process. Because we verify the table, we clean it up with leftover terms and try to fill TSR TAN from term name (in the future). We might need a word for this, that we both start using for this process.

)
)
Expand Down
15 changes: 10 additions & 5 deletions src/Client/Update/OfficeInteropUpdate.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ open OfficeInterop
open Shared
open OfficeInteropTypes
open Model
open ARCtrl

module OfficeInterop =
let update (state: OfficeInterop.Model) (model:Model) (msg: OfficeInterop.Msg) : OfficeInterop.Model * Model * Cmd<Messages.Msg> =
Expand Down Expand Up @@ -159,14 +158,14 @@ module OfficeInterop =
(fun tmin -> tmin |> Option.map (fun t -> ARCtrl.OntologyAnnotation.fromTerm t.toTerm) |> TermSearch.UpdateParentTerm |> TermSearchMsg)
(curry GenericError Cmd.none >> DevMsg)
state, model, cmd
//

| FillHiddenColsRequest ->
failwith "FillHiddenColsRequest Not implemented yet"
//failwith "FillHiddenColsRequest Not implemented yet"
//let cmd =
// Cmd.OfPromise.either
// OfficeInterop.Core.getAllAnnotationBlockDetails
// ()
// (fun (searchTerms,deprecationLogs) ->
// (fun (searchTerms, deprecationLogs) ->
// // Push possible deprecation messages by piping through "GenericInteropLogs"
// GenericInteropLogs (
// // This will be executed after "deprecationLogs" are handled by "GenericInteropLogs"
Expand All @@ -179,7 +178,13 @@ module OfficeInterop =
// (curry GenericError (UpdateFillHiddenColsState FillHiddenColsState.Inactive |> OfficeInteropMsg |> Cmd.ofMsg) >> DevMsg)
//let stateCmd = UpdateFillHiddenColsState FillHiddenColsState.ExcelCheckHiddenCols |> OfficeInteropMsg |> Cmd.ofMsg
//let cmds = Cmd.batch [cmd; stateCmd]
state, model, Cmd.none
let cmd =
Cmd.OfPromise.either
OfficeInterop.Core.fillEmptyBuildingBlocks
()
(curry GenericInteropLogs Cmd.none >> DevMsg)
(curry GenericError Cmd.none >> DevMsg)
state, model, cmd

| FillHiddenColumns (termsWithSearchResult) ->
let nextState = {
Expand Down
6 changes: 0 additions & 6 deletions src/Client/Update/SpreadsheetUpdate.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@ open Messages
open Elmish
open Spreadsheet
open LocalHistory
open Spreadsheet
open Model
open Shared
open Fable.Remoting.Client
open FsSpreadsheet
open FsSpreadsheet.Js
open ARCtrl
open ARCtrl.Spreadsheet
Expand All @@ -17,10 +15,6 @@ open ARCtrl.Json
module Spreadsheet =

module Helper =
open Browser
open Fable.Core
open Fable.Core.JS
open Fable.Core.JsInterop

let download(filename, bytes:byte []) = bytes.SaveFileAs(filename)

Expand Down
Loading