Expand file tokenisation #53
Conversation
There was a problem hiding this comment.
@LibraChris looks good, but i think we should separate the layers a bit cleaner.
- The API in
FileSystemshould provide a function that simply tokenizes a single URI (e.g. signatureSystem.Uri -> Cvparam) - That weay, the calling function can decide on the type of uris it provides, and we do not need to provide one function for paths and files each here.
- I like the idea of "traversing" the URI segments, and i think all these match cases could be unified to a single one
- These changes definately need some tests
src/ARCTokenization/Tokenization.fs
Outdated
| @@ -158,3 +159,40 @@ module Tokenization = | |||
| at.Columns | |||
| |> Array.map CompositeColumn.tokenize | |||
| |> List.ofArray | |||
|
|
|||
| module SpecificTokens = | |||
There was a problem hiding this comment.
rename SpecificTokens -> ArcFileSystem
src/ARCTokenization/FileSystem.fs
Outdated
|
|
||
|
|
||
| let private normalisePath (path:string) = |
There was a problem hiding this comment.
use internal instead of private
src/ARCTokenization/Tokenization.fs
Outdated
| | _ -> StructuralOntology.AFSO.``File Path`` |> fun t -> CvParam(t,relativePath) | ||
|
|
||
| /// Gets CvParams based on the root path, file system type, and full path | ||
| let getSpecificTokens (rootPath:string) (pType:PType) (path:string) = |
There was a problem hiding this comment.
rename getSpecificTokens -> getArcFileSystemTokens
src/ARCTokenization/Tokenization.fs
Outdated
| | Directory | ||
|
|
||
| /// Matches a CvParam based on the relative path and file system type | ||
| let matchFileSystemTokenByRelativePath (pType:PType) (relativePath: string) = |
There was a problem hiding this comment.
rename matchFileSystemTokenByRelativePath -> convertRelativePath
Add .gitkeep to arcStructureTests in oder to track folder
|
@kMutagene |
I would suggest by adding the root path as argument |
- Rework MetadataSheet parsers by using the new Tokens - Add parseProcessGraphColumnsFromToken and parseProcessGraphColumnsFromTokens
| @@ -9,54 +9,68 @@ module internal ISA = | |||
|
|
|||
| open System.IO | |||
|
|
|||
| let tryParseMetadataSheetFromToken (isaFileName: string) (isaMdsParsingF: string -> IParam list) (absFileToken: IParam) = | |||
| let tryParseMetadataSheetFromToken (rootPath:string) (isaCvTerm: CvTerm) (isaMdsParsingF: string -> IParam list) (absFileToken: IParam) = | |||
There was a problem hiding this comment.
nitpick: rename absFileToken -> relFileToken, since we ar enow tokenizing the farc file system as relative paths by convention
| with _ -> | ||
| None | ||
| else None | ||
|
|
||
| let parseMetadataSheetsFromTokens (isaFileName: string) (isaMdsParsingF: string -> IParam list) (absFileTokens: #IParam seq) = | ||
| let parseMetadataSheetsFromTokens (rootPath:string) (isaCvTerm: CvTerm) (isaMdsParsingF: string -> IParam list) (absFileTokens: #IParam seq) = |
| /// </summary> | ||
| /// <param name="rootPath">ARC root path</param> | ||
| /// <param name="absFileToken">he path to the study xlsx file</param> | ||
| let parseProcessGraphColumnsFromToken (rootPath:string) (absFileToken: IParam) = |
| /// <param name="rootPath">ARC root path</param> | ||
| /// <param name="isaCvTerm">The term to which the token should be found</param> | ||
| /// <param name="absFileToken">he path to the study xlsx file</param> | ||
| let parseProcessGraphColumnsFromTokens (rootPath:string) (isaCvTerm: CvTerm) (absFileTokens: #IParam seq) = |
There was a problem hiding this comment.
absFileTokens -> relFileTokens
| /// </summary> | ||
| /// <param name="rootPath">ARC root path</param> | ||
| /// <param name="absFileToken">he path to the study xlsx file</param> | ||
| static member parseProcessGraphColumnsFromToken (rootPath:string) (absFileToken: IParam) = |
| /// </summary> | ||
| /// <param name="rootPath">ARC root path</param> | ||
| /// <param name="absFileToken">he path to the xlsx file</param> | ||
| static member parseProcessGraphColumnsFromTokens (rootPath:string) (isaCsvTerm) (absFileTokens: #IParam seq) = |
There was a problem hiding this comment.
absFileTokens -> relFileTokens
| /// <param name="rootPath">ARC root path</param> | ||
| /// <param name="absFileToken">he path to the study xlsx file</param> | ||
| static member parseProcessGraphColumnsFromToken (rootPath:string) (absFileToken: IParam) = |
| /// </summary> | ||
| /// <param name="rootPath">ARC root path</param> | ||
| /// <param name="absFileToken">he path to the xlsx file</param> |
There was a problem hiding this comment.
absFileTokens -> relFileTokens
Adress the requested Changes
| | true -> Some (parseProcessGraphColumnsFromToken rootPath token) | ||
| | false -> None | ||
| ) | ||
| |> fun x -> |
There was a problem hiding this comment.
these functions should not fail but just return an empty sequence (maybe additionally printing warnings). Imagine if this is used in a validation package. It would lead to the parsing failing, without any validation being performed.
| refFileTokens | ||
| |> Seq.choose (fun token -> tryParseMetadataSheetFromToken rootPath isaCvTerm isaMdsParsingF token) | ||
|
|
||
| let parseProcessGraphColumnsFromToken (rootPath:string) (refFileToken: IParam) = |
There was a problem hiding this comment.
refFileToken
can you elaborate on that naming? i'd have gone with relFileToken or relativeFileToken
| let parseProcessGraphColumnsFromTokens (rootPath:string) (isaCvTerm: CvTerm) (refFileTokens: #IParam seq) = | ||
| refFileTokens | ||
| |> Seq.choose (fun token -> | ||
| match token.Name = isaCvTerm.Name with |
There was a problem hiding this comment.
i think we can do a full match here. There are terms out there with the same name from different ontologies.
| /// <param name="rootPath">ARC root path</param> | ||
| /// <param name="refFileToken">IParam seq of the ARC Tokens</param> | ||
| static member parseProcessGraphColumnsFromTokens (rootPath:string) (refFileTokens: #IParam seq) = |
There was a problem hiding this comment.
The xml comments still do not help that much.
IParam seq of the ARC Tokens
says nothing to me.
Also
Returns a seq of annotation tables from an IParam seq if the given tokens contains a filepath with the standard assay file name ("isa.assay.xlsx")
is only technically true, as we now match the term. So i would suggest something like this:
Returns a seq of annotation tables from an IParam seq for each contained token that is annotated with the term 'Assay File' (e.g, files in the ARC named
isa.assay.xlsx).
and for refFileTokens (typo there as well)
a seq of IParams that may contain relevant tokens
| ) | ||
| |> fun x -> | ||
| match Seq.length x with | ||
| | 0 -> failwith "No token found" | ||
| | 0 -> Seq.empty |
There was a problem hiding this comment.
convoluted way of just returning x :D
Expand the tokenisation of files and directories as described in #52.