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

Clean up measurements Redux states/actions #1881

Merged
merged 12 commits into from
Nov 6, 2024
Merged
41 changes: 15 additions & 26 deletions src/actions/loadData.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,13 @@ import { getServerAddress } from "../util/globals";
import { goTo404 } from "./navigation";
import { createStateFromQueryOrJSONs, createTreeTooState, getNarrativePageFromQuery } from "./recomputeReduxState";
import { loadFrequencies } from "./frequencies";
import { parseMeasurementsJSON, loadMeasurements } from "./measurements";
import { fetchJSON, fetchWithErrorHandling } from "../util/serverInteraction";
import { warningNotification, errorNotification } from "./notifications";
import { parseMarkdownNarrativeFile } from "../util/parseNarrative";
import { NoContentError, FetchError} from "../util/exceptions";
import { parseMarkdown } from "../util/parseMarkdown";
import { updateColorByWithRootSequenceData } from "../actions/colors";
import { explodeTree } from "./tree";
import { togglePanelDisplay } from "./panelDisplay";

export function getDatasetNamesFromUrl(url) {
let secondTreeUrl;
Expand Down Expand Up @@ -121,13 +119,15 @@ function narrativeFetchingErrorNotification(err) {
*/
async function dispatchCleanStart(dispatch, main, second, query, narrativeBlocks) {
const json = await main.main;
const measurementsData = main.measurements ? (await main.measurements) : undefined;
const secondTreeDataset = second ? (await second.main) : undefined;
const pathnameShouldBe = second ? `${main.pathname}:${second.pathname}` : main.pathname;
dispatch({
type: types.CLEAN_START,
pathnameShouldBe: narrativeBlocks ? undefined : pathnameShouldBe,
...createStateFromQueryOrJSONs({
json,
measurementsData,
secondTreeDataset,
query,
narrativeBlocks,
Expand Down Expand Up @@ -280,7 +280,19 @@ Dataset.prototype.fetchMain = function fetchMain() {
}
return res;
})
.then((res) => res.json());
.then((res) => res.json())
.then((json) => {
if (json.meta.panels && json.meta.panels.includes("measurements") && !this.measurements) {
/**
* Fetch measurements and store the resulting promise.
* Avoid the browser's default unhandled promise rejection logging and
* just resolve to an Error object that will be handled appropriately in loadMeasurements.
*/
this.measurements = fetchJSON(this.apiCalls.measurements)
.catch((reason) => Promise.resolve(reason));
}
return json;
});
};
Dataset.prototype.fetchSidecars = async function fetchSidecars() {
/**
Expand All @@ -304,12 +316,6 @@ Dataset.prototype.fetchSidecars = async function fetchSidecars() {
this.rootSequence = fetchJSON(this.apiCalls.rootSequence)
.catch((reason) => Promise.resolve(reason))
}

if (mainJson.meta.panels && mainJson.meta.panels.includes("measurements") && !this.measurements) {
this.measurements = fetchJSON(this.apiCalls.measurements)
.then((json) => parseMeasurementsJSON(json))
.catch((reason) => Promise.resolve(reason))
}
};
Dataset.prototype.loadSidecars = function loadSidecars(dispatch) {
/* Helper function to load (dispatch) the visualisation of sidecar files.
Expand Down Expand Up @@ -346,23 +352,6 @@ Dataset.prototype.loadSidecars = function loadSidecars(dispatch) {
dispatch(warningNotification({message: "Failed to parse root sequence JSON"}));
})
}
if (this.measurements) {
this.measurements
.then((data) => {
if (data instanceof Error) throw data;
return data
})
.then((data) => dispatch(loadMeasurements(data)))
.catch((err) => {
const errorMessage = `Failed to ${err instanceof FetchError ? 'fetch' : 'parse'} measurements collections`;
console.error(errorMessage, err.message);
dispatch(warningNotification({message: errorMessage}));
// Hide measurements panel
dispatch(togglePanelDisplay("measurements"));
// Save error message to display if user toggles panel again
dispatch({ type: types.UPDATE_MEASUREMENTS_ERROR, data: errorMessage });
});
}
};
Dataset.prototype.fetchAvailable = async function fetchAvailable() {
this.available = fetchJSON(this.apiCalls.getAvailable);
Expand Down
60 changes: 36 additions & 24 deletions src/actions/measurements.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { cloneDeep, pick } from "lodash";
import { measurementIdSymbol } from "../util/globals";
import { defaultMeasurementsControlState } from "../reducers/controls";
import { getDefaultMeasurementsState } from "../reducers/measurements";
import { warningNotification } from "./notifications";
import {
APPLY_MEASUREMENTS_FILTER,
CHANGE_MEASUREMENTS_COLLECTION,
CHANGE_MEASUREMENTS_DISPLAY,
CHANGE_MEASUREMENTS_GROUP_BY,
LOAD_MEASUREMENTS,
TOGGLE_MEASUREMENTS_OVERALL_MEAN,
TOGGLE_MEASUREMENTS_THRESHOLD,
} from "./types";
Expand Down Expand Up @@ -171,7 +172,7 @@ const getCollectionDisplayControls = (controls, collection) => {
return newControls;
};

export const parseMeasurementsJSON = (json) => {
const parseMeasurementsJSON = (json) => {
const collections = json["collections"];
if (!collections || collections.length === 0) {
throw new Error("Measurements JSON does not have collections");
Expand Down Expand Up @@ -263,34 +264,45 @@ export const parseMeasurementsJSON = (json) => {
);
});

return {collections, defaultCollection: json["default_collection"]};
};

export const loadMeasurements = ({collections, defaultCollection}) => (dispatch, getState) => {
const { tree, controls } = getState();
if (!tree.loaded) {
throw new Error("tree not loaded");
}

const collectionKeys = collections.map((collection) => collection.key);
let defaultCollectionKey = defaultCollection;
let defaultCollectionKey = json["default_collection"];
if (!collectionKeys.includes(defaultCollectionKey)) {
defaultCollectionKey = collectionKeys[0];
}

// Get the collection to display to set up default controls
const collectionToDisplay = getCollectionToDisplay(collections, controls.measurementsCollectionKey, defaultCollectionKey);
const newControls = getCollectionDisplayControls(controls, collectionToDisplay);
const queryParams = createMeasurementsQueryFromControls(newControls, collectionToDisplay, defaultCollectionKey);

dispatch({
type: LOAD_MEASUREMENTS,
const collectionToDisplay = collections.filter((collection) => collection.key === defaultCollectionKey)[0];
return {
loaded: true,
error: undefined,
defaultCollectionKey,
collections,
collectionToDisplay,
controls: newControls,
queryParams
});
collectionToDisplay
}
};

export const loadMeasurements = (measurementsData, dispatch) => {
let measurementState = getDefaultMeasurementsState();
let warningMessage = "";
if (measurementsData === undefined) {
// eslint-disable-next-line no-console
console.debug("No measurements JSON fetched");
joverlee521 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

For a dataset without measurements measurementsData=false so we're getting the warning message when we shouldn't. This happens because while the dispatch object sets measurementsData: undefined that won't override the default arg value of createStateFromQueryOrJSONs({measurementsData = false, ...}). This is a feature / bug of JS depending on your point of view:

function test({aaa=false}={}) {console.log("inside", aaa)}
test() // inside false
test({aaa: undefined}) // inside false

You can use null if you want, but I'd suggest false

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah good catch, I'll go with false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ended up just checking measurementsData is a falsy value in 0069e09.

} else if (measurementsData instanceof Error) {
console.error(measurementsData);
warningMessage = "Failed to fetch measurements collections";
} else {
try {
measurementState = { ...measurementState, ...parseMeasurementsJSON(measurementsData) };
} catch (error) {
console.error(error);
warningMessage = "Failed to parse measurements collections";
}
}

if (warningMessage) {
measurementState.error = warningMessage;
dispatch(warningNotification({ message: warningMessage }));
}

return measurementState;
};

export const changeMeasurementsCollection = (newCollectionKey) => (dispatch, getState) => {
Expand Down
2 changes: 2 additions & 0 deletions src/actions/navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,13 @@ const updateNarrativeDataset = async (dispatch, datasets, narrativeBlocks, path,
const mainDataset = datasets[mainTreeName];
const secondDataset = datasets[secondTreeName];
const mainJson = await mainDataset.main;
const measurementsData = mainDataset.measurements ? (await mainDataset.measurements) : undefined;
const secondJson = secondDataset ? (await secondDataset.main) : false;
dispatch({
type: URL_QUERY_CHANGE_WITH_COMPUTED_STATE,
...createStateFromQueryOrJSONs({
json: mainJson,
measurementsData,
secondTreeDataset: secondJson,
mainTreeName,
secondTreeName,
Expand Down
11 changes: 7 additions & 4 deletions src/actions/recomputeReduxState.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { getIdxMatchingLabel, calculateVisiblityAndBranchThickness } from "../ut
import { constructVisibleTipLookupBetweenTrees } from "../util/treeTangleHelpers";
import { getDefaultControlsState, shouldDisplayTemporalConfidence } from "../reducers/controls";
import { getDefaultFrequenciesState } from "../reducers/frequencies";
import { getDefaultMeasurementsState } from "../reducers/measurements";
import { countTraitsAcrossTree, calcTotalTipsInTree, gatherTraitNames } from "../util/treeCountingHelpers";
import { calcEntropyInView } from "../util/entropy";
import { treeJsonToState } from "../util/treeJsonProcessing";
Expand All @@ -23,7 +22,7 @@ import { getTraitFromNode, getDivFromNode, collectGenotypeStates } from "../util
import { collectAvailableTipLabelOptions } from "../components/controls/choose-tip-label";
import { hasMultipleGridPanels } from "./panelDisplay";
import { strainSymbolUrlString } from "../middleware/changeURL";
import { createMeasurementsControlsFromQuery, getCollectionDefaultControls, getCollectionToDisplay } from "./measurements";
import { createMeasurementsControlsFromQuery, getCollectionDefaultControls, getCollectionToDisplay, loadMeasurements } from "./measurements";

export const doesColorByHaveConfidence = (controlsState, colorBy) =>
controlsState.coloringsPresentOnTreeWithConfidence.has(colorBy);
Expand Down Expand Up @@ -858,6 +857,7 @@ export const getNarrativePageFromQuery = (query, narrative) => {

export const createStateFromQueryOrJSONs = ({
json = false, /* raw json data - completely nuke existing redux state */
measurementsData = false, /* raw measurements json data or error, only used when main json is provided */
secondTreeDataset = false,
oldState = false, /* existing redux state (instead of jsons) */
narrativeBlocks = false, /* if in a narrative this argument is set */
Expand All @@ -875,8 +875,8 @@ export const createStateFromQueryOrJSONs = ({
entropy = entropyCreateState(json.meta.genome_annotations);
/* ensure default frequencies state */
frequencies = getDefaultFrequenciesState();
/* ensure default measurements state */
measurements = getDefaultMeasurementsState();
/* Load measurements if available, otherwise ensure default measurements state */
measurements = loadMeasurements(measurementsData, dispatch);
/* new tree state(s) */
tree = treeJsonToState(json.tree);
castIncorrectTypes(metadata, tree);
Expand All @@ -891,6 +891,9 @@ export const createStateFromQueryOrJSONs = ({
controls = getDefaultControlsState();
controls = modifyControlsStateViaTree(controls, tree, treeToo, metadata.colorings);
controls = modifyStateViaMetadata(controls, metadata, entropy.genomeMap);
if (measurements.loaded) {
controls = {...controls, ...getCollectionDefaultControls(measurements.collectionToDisplay)};
}
} else if (oldState) {
/* creating deep copies avoids references to (nested) objects remaining the same which
can affect props comparisons. Due to the size of some of the state, we only do this selectively */
Expand Down
1 change: 1 addition & 0 deletions src/components/narrativeEditor/examineNarrative.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ const ExamineNarrative = ({narrative, datasetResponses, setDisplayNarrative}) =>
preserveCache: true, // specific for the narrative debugger
...createStateFromQueryOrJSONs({
json: await narrative.datasets[datasetNames[0]].main,
measurementsData: narrative.datasets[datasetNames[0]].measurements ? (await narrative.datasets[datasetNames[0]].measurements) : undefined,
secondTreeDataset: datasetNames[1] ? await narrative.datasets[datasetNames[1]].main : undefined,
query: n ? {n} : {}, // query is things like n=3 to load a specific page
narrativeBlocks: narrative.blocks,
Expand Down
11 changes: 2 additions & 9 deletions src/reducers/measurements.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {
CHANGE_MEASUREMENTS_COLLECTION,
LOAD_MEASUREMENTS,
CLEAN_START,
UPDATE_MEASUREMENTS_ERROR,
URL_QUERY_CHANGE_WITH_COMPUTED_STATE
} from "../actions/types";
Expand All @@ -15,16 +15,9 @@ export const getDefaultMeasurementsState = () => ({

const measurements = (state = getDefaultMeasurementsState(), action) => {
switch (action.type) {
case CLEAN_START: // fallthrough
case URL_QUERY_CHANGE_WITH_COMPUTED_STATE:
return { ...action.measurements };
case LOAD_MEASUREMENTS:
return {
...state,
loaded: true,
defaultCollectionKey: action.defaultCollectionKey,
collections: action.collections,
collectionToDisplay: action.collectionToDisplay
};
case CHANGE_MEASUREMENTS_COLLECTION:
return {
...state,
Expand Down