Skip to content

Commit

Permalink
Fixed ProcStudies NaN ProgressBar issue & removed BSup from Import Sc…
Browse files Browse the repository at this point in the history
…hemas
  • Loading branch information
MauricePasternak committed Aug 11, 2022
1 parent 6539528 commit 5b7014a
Show file tree
Hide file tree
Showing 12 changed files with 111 additions and 59 deletions.
10 changes: 9 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,23 @@ Versions are in format: `[MAJOR.MINOR.BUGFIX]`

Dates are in format: `YYYY-MM-DD`

## [0.2.2] - 2022-08-09
## [0.2.2] - 2022-08-10

Various improvements to the logic of the Data Visualization module and Dialog-based form components.

### Fixed

- Fixed a typo in calculateWorkload which caused string values to be concatenated to the numerical value of the
anticipated workload.

- Fixed a bug where multiple dialog windows could be opened for dialog-based components. A useRef was used to track
the dialog's open state to prevent this.

- Fixed a bug in Data Visualization where changing between plots did not reset all the plotting variables.

- Fixed a bug where additional hover data permitted the selection of variables that were already dedicated to X/Y axis,
causing a doubling effect to occur. This has been fixed twofold:

- Variables that are already dedicated to X/Y are disabled in the "additional hover data" section.
- Selecting a variable for X/Y that is already in "additional hover data" will cause it to be removed from the latter.

Expand All @@ -34,7 +38,11 @@ Various improvements to the logic of the Data Visualization module and Dialog-ba

### Changed

- Removed BackgroundSuppression from Import-related schemas, as it can be automatically derived from the number of
suppressions

- Hovering over datapoints in Data Visualization now has several changes:

- Subject and session are explicitly stated in the hoverdata instead of the previous ambiguous "ID" label
- Instead of X and Y, the variable names are now explicitly stated in the hoverlabel

Expand Down
2 changes: 1 addition & 1 deletion src/backend/runExploreASLHelperFunctions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ export async function calculateWorkload(
]);
payload = {
anticipatedFilepaths: [...strucResult.anticipatedFilepaths, ...aslResult.anticipatedFilepaths],
anticipatedWorkload: lodashSum([...strucResult.anticipatedWorkload, ...aslResult.anticipatedFilepaths]),
anticipatedWorkload: lodashSum([...strucResult.anticipatedWorkload, ...aslResult.anticipatedWorkload]),
};
/************
* Population
Expand Down
2 changes: 1 addition & 1 deletion src/backend/runImportModuleHelperFunctions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ export async function buildStudyParJSON(formValues: ImportSchemaType, whichConte
M0: importContext.M0IsSeparate,
PostLabelingDelay: importContext.PostLabelingDelay,
LabelingDuration: importContext.LabelingDuration,
BackgroundSuppression: importContext.BackgroundSuppression,
BackgroundSuppression: importContext.BackgroundSuppressionNumberPulses > 0,
BackgroundSuppressionNumberPulses: importContext.BackgroundSuppressionNumberPulses,
BackgroundSuppressionPulseTime: importContext.BackgroundSuppressionPulseTime,
};
Expand Down
43 changes: 39 additions & 4 deletions src/common/schemas/DataParSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
IsValidStudyRoot,
} from "../utilityFunctions/EASLFunctions";
import { YupShape } from "./ImportSchema";

const { api } = window;
const SchemaDataParDatasetParams: Yup.SchemaOf<DatasetParamsSchema> = Yup.object().shape({
name: Yup.string().typeError("Invalid value").default(""),
subjectRegexp: Yup.string().default(""),
Expand Down Expand Up @@ -88,8 +88,43 @@ const SchemaDataParGUIParams = Yup.object().shape<YupShape<GUIParamsSchema>>({
.typeError("This value must be a collection of folder names")
.of(Yup.string())
.min(1, "At least one subject is required")
.test("AreValidSubjects", async (subjects, helpers: Yup.TestContext<DataParValuesType>) => {
return await AreValidSubjects(subjects, helpers);
.test("AreValidSubjects", async (subjectBasenames, helpers: Yup.TestContext<DataParValuesType>) => {
console.log(`DataParSchema -- SUBJECTS field -- subjectBasenames`, subjectBasenames);
if (!subjectBasenames || !Array.isArray(subjectBasenames) || !subjectBasenames.length) {
return helpers.createError({
path: helpers.path,
message: "Invalid value provided for the listing of subjects",
});
}

// Must first ascertain that
const StudyRootPath: string | undefined = helpers.options.context.x.GUI.StudyRootPath;
console.log(`DataParSchema -- SUBJECTS field -- StudyRootPath`, helpers.options.context.x.GUI.StudyRootPath);

if (
!StudyRootPath || // Cannot be falsy
typeof StudyRootPath !== "string" || // Must be a string
(await IsValidStudyRoot(StudyRootPath, helpers, ["sourcedata", "rawdata", "derivatives"])) !== true // Must be a valid study root
) {
return helpers.createError({
path: helpers.path,
message: "Cannot validate the subjects because the Study Root Path itself is invalid",
});
}

// Must all exist in rawdata
const existenceChecks = await api.path.getFilepathsType(
subjectBasenames.map(subjectBasename => `${StudyRootPath}/rawdata/${subjectBasename}`)
);
console.log(`DataParSchema -- SUBJECTS field -- existenceChecks`, existenceChecks);
if (!existenceChecks.every(check => check === "dir")) {
return helpers.createError({
path: helpers.path,
message: "One or more of the provided subjects do not exist in the rawdata folder",
});
}

return true;
}),
});

Expand Down Expand Up @@ -290,7 +325,7 @@ const SchemaDataParASLQuantificationParams = Yup.object().shape<YupShape<ASLQuan
.test(
"ApplyQuantificationIsValid",
"These should be a collection of five numbers, each of which can be 0 or 1",
(value) => {
value => {
if (!Array.isArray(value) || !value.every(v => v === 0 || v === 1)) {
return false;
}
Expand Down
50 changes: 27 additions & 23 deletions src/common/schemas/ImportSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,10 @@ export const SchemaImportDefineContext = Yup.object().shape<YupShape<ImportConte
// For the global context, there should be no subjectPaths
const isGlobal = helpers.parent.IsGlobal ?? true;

// console.log("Validating Subjects -- isGlobal: ", isGlobal, subjectPaths);
// console.log("SchemaImportDefineContext -- Subjects -- isGlobal: ", {
// isGlobal,
// subjectPaths,
// });

if (isGlobal && subjectPaths.length === 0) return true;

Expand All @@ -191,12 +194,16 @@ export const SchemaImportDefineContext = Yup.object().shape<YupShape<ImportConte
const globStars = SourcedataStructure.slice(0, nLevels + 1)
.map(() => "*")
.join("/");

console.log(`Searching for subjects at fullPath Pattern ${studyRootPath}/sourcedata/${globStars}`);

const foundSubjectPaths = new Set(
(await api.path.glob(`${studyRootPath}/sourcedata`, globStars, { onlyDirectories: true })).map(p => p.path)
);

for (const subjectPath of subjectPaths) {
if (!foundSubjectPaths.has(subjectPath))
const asPath = api.path.asPath(subjectPath); // Necessary to normalize to forward slashes in Windows
if (!foundSubjectPaths.has(asPath.path))
return helpers.createError({
path: helpers.path,
message: `Subject ${subjectPath} does not exist in the folder structure you had previously specified`,
Expand Down Expand Up @@ -264,27 +271,24 @@ export const SchemaImportDefineContext = Yup.object().shape<YupShape<ImportConte
ASLSequence: Yup.string().optional().oneOf(["PASL", "CASL", "PCASL"], "Invalid ASL Sequence"),
PostLabelingDelay: Yup.number().optional(),
LabelingDuration: Yup.number().optional(),
BackgroundSuppression: Yup.boolean(),
BackgroundSuppressionNumberPulses: Yup.number().test(
"ValidNumberOfSuppressionPulses",
"Invalid Number of Pulses",
(numPulses, helpers) => {
const hasSuppression = helpers.parent.BackgroundSuppression;

if (hasSuppression && numPulses === 0)
return helpers.createError({
path: helpers.path,
message: "Background suppression could not have been used when there were no pulses",
});
if (!hasSuppression && numPulses > 0)
return helpers.createError({
path: helpers.path,
message: "Number of pulses must be 0 when background suppression is disabled",
});
return true;
}
),
BackgroundSuppressionPulseTime: Yup.array().of(Yup.number().integer("Must be an integer")).optional(),
BackgroundSuppressionNumberPulses: Yup.number().integer("Must be an integer"),
BackgroundSuppressionPulseTime: Yup.array().when("BackgroundSuppressionNumberPulses", {
is: (nPulses: number) => nPulses > 0,
then: Yup.array()
.of(Yup.number())
.test(
"ValidPulseTimes",
"If provided, the number of timings specified must match number of pulses in the other field",
(pulseTimes, helpers: Yup.TestContext<ImportSchemaType>) => {
if (pulseTimes.length === 0) return true;
const nPulses: number = helpers.parent.BackgroundSuppressionNumberPulses;
return pulseTimes.length === nPulses;
}
),
otherwise: Yup.array()
.of(Yup.number())
.max(0, "This field must be empty if there are no background suppression pulses indicated"),
}),
});

export const SchemaImportStepDefineMultiContext = Yup.object().shape<YupShape<ImportMultipleContextsSchemaType>>({
Expand Down
1 change: 0 additions & 1 deletion src/common/types/ImportSchemaTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ export type ImportContextSchemaType = {
ASLSequence: ASLSequenceType;
PostLabelingDelay?: number;
LabelingDuration?: number;
BackgroundSuppression?: boolean;
BackgroundSuppressionNumberPulses?: number;
BackgroundSuppressionPulseTime?: number[];
};
Expand Down
12 changes: 6 additions & 6 deletions src/common/utilityFunctions/EASLFunctions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { FieldValues } from "react-hook-form";
import { TestContext } from "yup";
import { SUPPORTEDMATLABRUNTIMEVERSIONS } from "../GLOBALS";
import { EASLWorkloadMapping } from "../schemas/ExploreASLWorkloads";
import { DataParValuesType } from "../types/ExploreASLDataParTypes";
import { EASLType } from "../types/ImportSchemaTypes";
const { api } = window;

Expand Down Expand Up @@ -98,6 +99,7 @@ export async function IsValidStudyRoot(
helpers: TestContext,
expectedChildren: string[] = ["sourcedata"]
) {
console.log("IsValidStudyRoot", filepath);
// Filepath must be a valid string
if (!filepath) return false;

Expand All @@ -123,7 +125,7 @@ export async function IsValidStudyRoot(
* @param helpers Helpers context & functions provided by Yup.
* @returns A boolean or a Yup.ValidationError.
*/
export async function AreValidSubjects(subjectBasenames: string[], helpers: TestContext) {
export async function AreValidSubjects(subjectBasenames: string[], helpers: TestContext<DataParValuesType>) {
if (!subjectBasenames || !Array.isArray(subjectBasenames) || !subjectBasenames.length) {
return helpers.createError({
path: helpers.path,
Expand All @@ -132,10 +134,8 @@ export async function AreValidSubjects(subjectBasenames: string[], helpers: Test
}

// Must first ascertain that
const StudyRootPath: string | undefined = helpers.parent.StudyRootPath;

// const test = await IsValidStudyRoot(StudyRootPath, helpers, ["sourcedata", "rawdata", "derivatives"]);
// console.log("test", test);
const StudyRootPath: string | undefined = helpers.options.context.x.GUI.StudyRootPath;
console.log(`AreValidSubjects: StudyRootPath: ${StudyRootPath}`);

if (
!StudyRootPath || // Cannot be falsy
Expand All @@ -160,4 +160,4 @@ export async function AreValidSubjects(subjectBasenames: string[], helpers: Test
}

return true;
}
}
7 changes: 4 additions & 3 deletions src/components/FormComponents/RHFFilepathDropzone.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, { useRef, useState, useEffect } from "react";
import { OpenDialogOptions } from "electron";
import { Controller, FieldValues, Path, PathValue } from "react-hook-form";
import { Controller, FieldValues, Path, PathValue, UseFormStateReturn } from "react-hook-form";
import { styled } from "@mui/material/styles";
import Paper from "@mui/material/Paper";
import FormControl, { FormControlProps } from "@mui/material/FormControl";
Expand Down Expand Up @@ -96,7 +96,7 @@ type ControlledFilepathDropzoneProps<
type ControlledFilepathDropzonePropsNoField<
TValues extends FieldValues,
TName extends Path<TValues> = Path<TValues>
> = Omit<ControlledFilepathDropzoneProps<TValues, TName>, "field" | "fieldState">;
> = Omit<ControlledFilepathDropzoneProps<TValues, TName>, "field" | "fieldState" | "formState">;

type RHFFilepathDropzoneProps<
TValues extends FieldValues,
Expand Down Expand Up @@ -170,6 +170,7 @@ function ControlledFilepathDropzone<TValues extends FieldValues, TName extends P
// innerVal
// );


useEffect(() => {
if (field.value != null && !FastIsEqual(field.value, innerVal)) {
setInnerVal(() => (handleFieldToInnerVal ? handleFieldToInnerVal(field.value) : field.value));
Expand Down Expand Up @@ -422,7 +423,7 @@ function RHFFilepathDropzone<TValues extends FieldValues, TName extends Path<TVa
<Controller
control={control}
name={name}
render={({ field, fieldState }) => {
render={({ field, fieldState, formState }) => {
return (
<ControlledFilepathDropzone field={field} fieldState={fieldState} {...controlledFilepathDropzoneProps} />
);
Expand Down
12 changes: 7 additions & 5 deletions src/components/FormComponents/RHFInterDepFilepathTextfield.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import IconButton from "@mui/material/IconButton";
import TextField, { TextFieldProps } from "@mui/material/TextField";
import { OpenDialogOptions } from "electron";
import React, { useCallback, useEffect, useRef, useState } from "react";
import { Controller, FieldValues, Path } from "react-hook-form";
import { Controller, FieldValues, Path, UseFormStateReturn } from "react-hook-form";
import { useDebouncedCallback } from "use-debounce";
import { RHFFieldAndFieldStateType, RHFInterDepBaseProps, RHFTriggerType } from "../../common/types/formTypes";

Expand All @@ -29,12 +29,12 @@ type InterDepControlledFilepathTextFieldProps<
> = RHFFieldAndFieldStateType<TValues, TName> & // field and fieldState
RHFTriggerType<TValues, TName> & // trigger and triggerTarget
ControlledInterDepFPTextFieldBaseProps &
RestrictedTextFieldProps;
RestrictedTextFieldProps & { formState: UseFormStateReturn<TValues> };

type ControlledFilepathTextFieldPropsNoField<
TValues extends FieldValues,
TName extends Path<TValues> = Path<TValues>
> = Omit<InterDepControlledFilepathTextFieldProps<TValues, TName>, "field" | "fieldState">;
> = Omit<InterDepControlledFilepathTextFieldProps<TValues, TName>, "field" | "fieldState" | "formState">;

type RHFFilepathTextFieldProps<
TValues extends FieldValues,
Expand All @@ -47,6 +47,7 @@ export function InterDepControlledFilepathTextField<
>({
field,
fieldState,
formState,
trigger,
triggerTarget,
variant,
Expand Down Expand Up @@ -105,7 +106,7 @@ export function InterDepControlledFilepathTextField<
*/
const debouncedHandleChange = useDebouncedCallback((values: any) => {
field.onChange(values);
trigger(triggerTarget);
formState.submitCount > 0 && trigger(triggerTarget);
}, debounceTime);

/**
Expand Down Expand Up @@ -257,11 +258,12 @@ function RHFInterDepFilepathTextField<TValues extends FieldValues, TName extends
<Controller
control={control}
name={name}
render={({ field, fieldState }) => {
render={({ field, fieldState, formState }) => {
return (
<InterDepControlledFilepathTextField
field={field}
fieldState={fieldState}
formState={formState}
{...controlledFilepathTextFieldProps}
/>
);
Expand Down
19 changes: 7 additions & 12 deletions src/pages/ImportModule/SingleImportContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import IconButton from "@mui/material/IconButton";
import Typography from "@mui/material/Typography";
import { partialRight as lodashPartialRight, range as lodashRange } from "lodash";
import React from "react";
import { Control, UseFieldArrayRemove } from "react-hook-form";
import { Control, UseFieldArrayRemove, UseFormTrigger } from "react-hook-form";
import RHFInterDepSlider from "../../components/FormComponents/RHFInterDepSlider";
import { ImportSchemaType } from "../../common/types/ImportSchemaTypes";
import { getNumbersFromDelimitedString } from "../../common/utilityFunctions/stringFunctions";
import ExpandMore from "../../components/ExpandMore";
Expand All @@ -26,9 +27,10 @@ type SingleImportContextProps = {
contextIndex: number;
control: Control<ImportSchemaType>;
remove: UseFieldArrayRemove;
trigger: UseFormTrigger<ImportSchemaType>;
};

function SingleImportContext({ contextIndex, control, remove }: SingleImportContextProps) {
function SingleImportContext({ contextIndex, control, remove, trigger }: SingleImportContextProps) {
const isFirst = contextIndex === 0;
const [expanded, setExpanded] = React.useState(true);

Expand Down Expand Up @@ -206,18 +208,11 @@ function SingleImportContext({ contextIndex, control, remove }: SingleImportCont
/>
</Grid>
<Grid item xs={12} md={6} xl={3}>
<RHFSingleCheckable
control={control}
name={`ImportContexts.${contextIndex}.BackgroundSuppression`}
label="Background Suppression was used"
valWhenChecked={true}
valWhenUnchecked={false}
/>
</Grid>
<Grid item xs={12} md={6} xl={3}>
<RHFSlider
<RHFInterDepSlider
control={control}
name={`ImportContexts.${contextIndex}.BackgroundSuppressionNumberPulses`}
trigger={trigger}
triggerTarget={`ImportContexts.${contextIndex}.BackgroundSuppressionPulseTime`}
label="Number of Background Suppression Pulses"
min={0}
max={10}
Expand Down
Loading

0 comments on commit 5b7014a

Please sign in to comment.