Cleanup typing and imports and formatting.

jmchilton · Jan 29, 2024 · 4217b3e · 4217b3e
1 parent 6b63ca9
commit 4217b3e
Show file tree

Hide file tree

Showing 10 changed files with 76 additions and 69 deletions.
diff --git a/client/src/components/Tool/ToolForm.vue b/client/src/components/Tool/ToolForm.vue
@@ -99,15 +99,15 @@
 </template>
 
 <script>
-import { getAppRoot } from "onload/loadConfig";
-import axios from "axios";
 import { getGalaxyInstance } from "app";
+import axios from "axios";
 import ButtonSpinner from "components/Common/ButtonSpinner";
 import Heading from "components/Common/Heading";
 import FormDisplay from "components/Form/FormDisplay";
 import FormElement from "components/Form/FormElement";
 import LoadingSpan from "components/LoadingSpan";
 import ToolEntryPoints from "components/ToolEntryPoints/ToolEntryPoints";
+import { getAppRoot } from "onload/loadConfig";
 import { mapActions, mapState, storeToRefs } from "pinia";
 import { useHistoryItemsStore } from "stores/historyItemsStore";
 import { useJobStore } from "stores/jobStore";
@@ -118,9 +118,9 @@ import { useHistoryStore } from "@/stores/historyStore";
 import { useUserStore } from "@/stores/userStore";
 
 import ToolRecommendation from "../ToolRecommendation";
-import { getToolFormData, getToolInputs, updateToolFormData, submitJob, submitToolRequest } from "./services";
-import ToolCard from "./ToolCard";
+import { getToolFormData, getToolInputs, submitJob, submitToolRequest, updateToolFormData } from "./services";
 import { structuredInputs } from "./structured";
+import ToolCard from "./ToolCard";
 import { allowCachedJobs } from "./utilities";
 
 export default {

diff --git a/client/src/components/Tool/ToolSuccess.vue b/client/src/components/Tool/ToolSuccess.vue
@@ -35,7 +35,7 @@ onMounted(() => {
         <div v-if="jobResponse.produces_entry_points">
             <ToolEntryPoints v-for="job in jobResponse.jobs" :key="job.id" :job-id="job.id" />
         </div>
-        <ToolSuccessMessage :job-response="jobResponse" :tool-name="toolName" :used-tool-request="usedToolRequest"/>
+        <ToolSuccessMessage :job-response="jobResponse" :tool-name="toolName" :used-tool-request="usedToolRequest" />
         <Webhook type="tool" :tool-id="jobDef.tool_id" />
         <ToolRecommendation v-if="showRecommendation" :tool-id="jobDef.tool_id" />
     </section>

diff --git a/client/src/components/Tool/structured.test.ts b/client/src/components/Tool/structured.test.ts
@@ -1,49 +1,51 @@
-import { structuredInputs, validate } from "./structured";
+import SPEC_PARAMETERS from "./parameter_models.yml";
+import SPEC_TESTS from "./parameter_specification.yml";
 import { TextParameterModel, ToolParameterModel } from "./parameterModels";
+import { structuredInputs, validate } from "./structured";
 
 describe("structured.js", () => {
     it("should parse galaxy integer parameters", () => {
         expect(true).toBe(true);
         const si = structuredInputs({ parameter: "5" }, [
             { name: "parameter", parameter_type: "gx_integer", optional: false },
         ]);
-        expect(si["parameter"]).toBe(5);
+        expect(si).toHaveProperty("parameter");
+        if ("parameter" in si) {
+            const val = si["parameter"];
+            expect(val).toBe(5);
+        }
     });
 });
 
-import SPEC_PARAMETERS from "./parameter_models.yml";
-import SPEC_TESTS from "./parameter_specification.yml";
+type TestCase = { [ParameterName: string]: any };
 
-class FileTestCases {
-    request_invalid: Array<any>;
-    request_valid: Array<any>;
+interface FileTestCases {
+    request_invalid: Array<TestCase>;
+    request_valid: Array<TestCase>;
 }
 
-function itShouldValidateParameters(file, parameters) {
+type ParameterSpecification = { [FileName: string]: FileTestCases };
+
+function itShouldValidateParameters(file: string, parameters: Array<TestCase>) {
     for (const [index, parameter] of parameters.entries()) {
         itShouldValidateParameter(file, index, parameter);
     }
 }
 
-function itShouldInvalidateParameters(file, parameters) {
+function itShouldInvalidateParameters(file: string, parameters: Array<TestCase>) {
     for (const [index, parameter] of parameters.entries()) {
         itShouldInvalidateParameter(file, index, parameter);
     }
 }
 
-function toToolParameterModel(jsonObject): ToolParameterModel {
-    const modal: TextParameterModel = jsonObject;
-    return modal;
-}
-
 function parameterModelsForFile(filename: string): Array<ToolParameterModel> {
     const parameterModel = SPEC_PARAMETERS[filename];
-    const parameterObject: ToolParameterModel = toToolParameterModel(parameterModel);
+    const parameterObject: ToolParameterModel = parameterModel as TextParameterModel;
     const inputs = [parameterObject];
     return inputs;
 }
 
-function itShouldValidateParameter(file, index, parameterTestCase) {
+function itShouldValidateParameter(file: string, index: number, parameterTestCase: TestCase) {
     let doc = " for file [" + file + "] and valid parameter combination [" + index + "]";
     if (parameterTestCase._doc) {
         doc = " - " + parameterTestCase._doc;
@@ -54,7 +56,7 @@ function itShouldValidateParameter(file, index, parameterTestCase) {
     });
 }
 
-function itShouldInvalidateParameter(file, index, parameterTestCase) {
+function itShouldInvalidateParameter(file: string, index: number, parameterTestCase: TestCase) {
     let doc = " for file [" + file + "] and invalid parameter combination [" + index + "]";
     if (parameterTestCase._doc) {
         doc = " - " + parameterTestCase._doc;
@@ -66,8 +68,7 @@ function itShouldInvalidateParameter(file, index, parameterTestCase) {
 }
 
 describe("Tool Parameter Specification", () => {
-    for (const [file, combos] of Object.entries(SPEC_TESTS)) {
-        const testCases: FileTestCases = Object.assign(new FileTestCases(), combos);
+    for (const [file, testCases] of Object.entries(SPEC_TESTS as ParameterSpecification)) {
         if (testCases.request_valid) {
             itShouldValidateParameters(file, testCases.request_valid);
         }

diff --git a/client/src/components/Tool/structured.ts b/client/src/components/Tool/structured.ts
@@ -21,16 +21,19 @@ import {
     ToolParameterModel,
 } from "./parameterModels";
 
-export function structuredInputs(formInputs: object, toolInputs: Array<ToolParameterModel>): object {
-    const structuredInputs = {};
+type StructuredInputs = { [parameterName: string]: any };
+type FormInputs = { [parameterName: string]: any };
+
+export function structuredInputs(formInputs: FormInputs, toolInputs: Array<ToolParameterModel>): StructuredInputs {
+    const structuredInputs: StructuredInputs = {};
     for (const toolInput of toolInputs) {
         const inputKey = toolInput.name;
         structuredInputs[inputKey] = parseInt(formInputs[inputKey]);
     }
     const validationResult = validateParameters(structuredInputs, toolInputs);
     if (validationResult.length !== 0) {
         console.log(`Failed structured input validation with... '${validationResult}'`);
-        throw "Failed parameter validation";
+        throw Error("Failed parameter validation");
     }
     return structuredInputs;
 }
@@ -125,7 +128,7 @@ function isCwlFloat(model: ToolParameterModel): model is CwlFloatParameterModel
     return model.parameter_type == "cwl_float";
 }
 
-const isBool = (v) => {
+const isBool = (v: any) => {
     return typeof v == "boolean";
 };
 
@@ -149,11 +152,13 @@ function isObjectWithKeys(inputObject: any, requiredKeys: Array<string>): boolea
     return true;
 }
 
-function isSrcReferenceObject(v, srcTypes: Array<string>) {
+function isSrcReferenceObject(v: any, srcTypes: Array<string>) {
     return isObjectWithKeys(v, ["src", "id"]) && srcTypes.indexOf(v.src) >= 0 && isString(v.id);
 }
 
-function isArrayOf(inputObject: any, typeCheck): boolean {
+type TypeChecker = (v: any) => boolean;
+
+function isArrayOf(inputObject: any, typeCheck: TypeChecker): boolean {
     if (!Array.isArray(inputObject)) {
         return false;
     }
@@ -165,11 +170,11 @@ function isArrayOf(inputObject: any, typeCheck): boolean {
     return true;
 }
 
-const isDataDict = (v) => {
+const isDataDict = (v: any) => {
     return isSrcReferenceObject(v, ["hda", "ldda"]);
 };
 
-const isBatch = (v, valueChecker) => {
+const isBatch = (v: any, valueChecker: TypeChecker) => {
     if (!isObjectWithKeys(v, ["__class__", "values"]) || v["__class__"] != "Batch") {
         return false;
     }
@@ -186,7 +191,7 @@ function simpleCwlTypeChecker(parameterModel: ToolParameterModel) {
     } else if (isCwlFile(parameterModel)) {
         checkType = isDataDict;
     } else if (isCwlNull(parameterModel)) {
-        checkType = (v) => {
+        checkType = (v: any) => {
             return v == null;
         };
     } else if (isCwlBoolean(parameterModel)) {
@@ -196,12 +201,12 @@ function simpleCwlTypeChecker(parameterModel: ToolParameterModel) {
     } else if (isCwlFloat(parameterModel)) {
         checkType = isFloat;
     } else {
-        throw "Unknown simple CWL type encountered.";
+        throw Error("Unknown simple CWL type encountered.");
     }
     return checkType;
 }
 
-function checkCwlUnionType(parameterModel: CwlUnionParameterModel, inputValue) {
+function checkCwlUnionType(parameterModel: CwlUnionParameterModel, inputValue: any) {
     for (const unionedModel of parameterModel.parameters) {
         if (simpleCwlTypeChecker(unionedModel)(inputValue)) {
             return true;
@@ -210,13 +215,13 @@ function checkCwlUnionType(parameterModel: CwlUnionParameterModel, inputValue) {
     return false;
 }
 
-function validateParameter(inputKey: string, inputValue, parameterModel: ToolParameterModel) {
-    const results = [];
+function validateParameter(inputKey: string, inputValue: any, parameterModel: ToolParameterModel) {
+    const results: string[] = [];
     let checkType = null;
 
-    function handleOptional(typeCheck, parameter) {
-        if (parameter.optional) {
-            return (v) => {
+    function handleOptional(typeCheck: TypeChecker, parameter: ToolParameterModel) {
+        if ("optional" in parameter && parameter.optional) {
+            return (v: any) => {
                 return v === null || typeCheck(v);
             };
         } else {
@@ -235,55 +240,55 @@ function validateParameter(inputKey: string, inputValue, parameterModel: ToolPar
     } else if (isGxHidden(parameterModel)) {
         checkType = handleOptional(isString, parameterModel);
     } else if (isGxColor(parameterModel)) {
-        const isColorString = (v) => {
+        const isColorString = (v: any) => {
             return isString(v) && /^#[0-9A-F]{6}$/i.test(v);
         };
         checkType = handleOptional(isColorString, parameterModel);
     } else if (isGxData(parameterModel)) {
-        const isMultiDataDict = (v) => {
+        const isMultiDataDict = (v: any) => {
             return isSrcReferenceObject(v, ["hda", "ldda", "hdca"]);
         };
-        const isArrayOfDataDict = (v) => {
+        const isArrayOfDataDict = (v: any) => {
             return isArrayOf(v, isMultiDataDict);
         };
-        const isBatchData = (v) => {
+        const isBatchData = (v: any) => {
             return isBatch(v, isMultiDataDict);
         };
-        let checkRaw;
+        let checkRaw: TypeChecker;
         if (parameterModel.multiple) {
             checkRaw = handleOptional((v) => {
                 return isMultiDataDict(v) || isArrayOfDataDict(v);
             }, parameterModel);
         } else {
             checkRaw = isDataDict;
         }
-        checkType = (v) => {
+        checkType = (v: any) => {
             return checkRaw(v) || isBatchData(v);
         };
         checkType = handleOptional(checkType, parameterModel);
     } else if (isGxSelect(parameterModel)) {
-        let isElement;
+        let isElement: TypeChecker;
         if (parameterModel.options != null) {
             const optionValues = parameterModel.options.map((lv) => {
                 return lv.value;
             });
-            const isOneOfOptions = (v) => {
+            const isOneOfOptions = (v: any) => {
                 return isString(v) && optionValues.indexOf(v) !== -1;
             };
             isElement = isOneOfOptions;
         } else {
             isElement = isString;
         }
         if (parameterModel.multiple) {
-            checkType = (v) => {
+            checkType = (v: any) => {
                 return isArrayOf(v, isElement);
             };
         } else {
             checkType = isElement;
         }
         checkType = handleOptional(checkType, parameterModel);
     } else if (isGxDataCollection(parameterModel)) {
-        const isDataCollectionDict = (v) => {
+        const isDataCollectionDict = (v: any) => {
             return isSrcReferenceObject(v, ["hdca"]);
         };
         checkType = handleOptional(isDataCollectionDict, parameterModel);
@@ -294,7 +299,7 @@ function validateParameter(inputKey: string, inputValue, parameterModel: ToolPar
     } else if (isCwlFile(parameterModel)) {
         checkType = simpleCwlTypeChecker(parameterModel);
     } else if (isCwlUnion(parameterModel)) {
-        checkType = (v) => {
+        checkType = (v: any) => {
             return checkCwlUnionType(parameterModel, v);
         };
     } else if (isCwlBoolean(parameterModel)) {
@@ -340,10 +345,13 @@ function validateParameter(inputKey: string, inputValue, parameterModel: ToolPar
     return results;
 }
 
-function validateParameters(structuredInputs: object, parameterModels: Array<ToolParameterModel>): Array<string> {
+function validateParameters(
+    structuredInputs: StructuredInputs,
+    parameterModels: Array<ToolParameterModel>
+): Array<string> {
     const results = [];
     const keysEncountered = [];
-    const parameterModelsByName = {};
+    const parameterModelsByName: { [name: string]: ToolParameterModel } = {};
     parameterModels.forEach((v) => {
         parameterModelsByName[v.name] = v;
     });
@@ -355,16 +363,18 @@ function validateParameters(structuredInputs: object, parameterModels: Array<Too
         }
         const inputValue = structuredInputs[inputKey];
         const parameterModel = parameterModelsByName[inputKey];
-        const parameterResults = validateParameter(inputKey, inputValue, parameterModel);
-        extendValidationResults(results, parameterResults);
+        if (parameterModel) {
+            const parameterResults = validateParameter(inputKey, inputValue, parameterModel);
+            extendValidationResults(results, parameterResults);
+        }
     }
     for (const parameterModel of parameterModels) {
         const inputKey = parameterModel.name;
         if (keysEncountered.indexOf(inputKey) !== -1) {
             continue;
         }
         const toolInput = parameterModelsByName[inputKey];
-        if (toolInput.optional === true) {
+        if ("optional" in toolInput && toolInput.optional === true) {
             continue;
         }
         results.push(`Non optional parameter ${inputKey} was not found in inputs.`);

diff --git a/lib/galaxy/tool_util/parameters/_types.py b/lib/galaxy/tool_util/parameters/_types.py
@@ -18,10 +18,8 @@
 # https://stackoverflow.com/questions/56832881/check-if-a-field-is-typing-optional
 # Python >= 3.8
 try:
-    from typing import (
-        get_args,  # type: ignore[attr-defined]
-        get_origin,  # type: ignore[attr-defined]
-    )
+    from typing import get_args  # type: ignore[attr-defined]
+    from typing import get_origin  # type: ignore[attr-defined]
 # Compatibility
 except ImportError:
 

diff --git a/lib/galaxy/tool_util/parameters/factory.py b/lib/galaxy/tool_util/parameters/factory.py
@@ -133,7 +133,7 @@ def _from_input_source_galaxy(input_source: InputSource) -> ToolParameterT:
             options: Optional[List[LabelValue]] = None
             if is_static:
                 options = []
-                for (option_label, option_value, selected) in input_source.parse_static_options():
+                for option_label, option_value, selected in input_source.parse_static_options():
                     options.append(LabelValue(label=option_label, value=option_value, selected=selected))
             return SelectParameterModel(
                 name=input_source.parse_name(),
@@ -151,7 +151,7 @@ def _from_input_source_galaxy(input_source: InputSource) -> ToolParameterT:
         if isinstance(test_parameter, BooleanParameterModel):
             default_value = test_parameter.value
         # TODO: handle select parameter model...
-        for (value, case_inputs_sources) in input_source.parse_when_input_sources():
+        for value, case_inputs_sources in input_source.parse_when_input_sources():
             if isinstance(test_parameter, BooleanParameterModel):
                 # TODO: investigate truevalue/falsevalue when...
                 from galaxy.util import string_as_bool

diff --git a/lib/galaxy/tool_util/parameters/models.py b/lib/galaxy/tool_util/parameters/models.py
@@ -692,7 +692,6 @@ def py_type(self) -> Type:
         return union_type(union_of_cwl_types)
 
     def pydantic_template(self, state_representation: StateRepresentationT) -> DynamicModelInformation:
-
         return DynamicModelInformation(
             self.name,
             (self.py_type, ...),
@@ -785,7 +784,6 @@ class ToolParameterBundle(Protocol):
 
 
 class ToolParameterBundleModel(BaseModel):
-
     input_models: List[ToolParameterT]
 
 

diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py
@@ -49,7 +49,7 @@
 from galaxy.model import (
     Job,
     StoredWorkflow,
-    ToolRequest
+    ToolRequest,
 )
 from galaxy.model.base import transaction
 from galaxy.model.dataset_collections import matching