From 633ef5d8b71b350d3613512a47d1646751117760 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Wed, 15 Nov 2023 09:19:58 -0300 Subject: [PATCH 1/7] Make data exploration diff expr work with an array of filters by types instead of a single one and add CLM and CLMPerSample to them Signed-off-by: cosa65 --- .../DiffExprCompute.jsx | 18 +++++++++--------- .../DiffExprSelect.jsx | 11 +++++++---- .../metadata/AddMetadataButton.jsx | 2 +- src/utils/cellSets/composeTree.js | 8 ++++---- 4 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/components/data-exploration/differential-expression-tool/DiffExprCompute.jsx b/src/components/data-exploration/differential-expression-tool/DiffExprCompute.jsx index a043084f32..e586cccd53 100644 --- a/src/components/data-exploration/differential-expression-tool/DiffExprCompute.jsx +++ b/src/components/data-exploration/differential-expression-tool/DiffExprCompute.jsx @@ -100,7 +100,7 @@ const DiffExprCompute = (props) => { * Constructs a form item, a `Select` field with selectable clusters. */ const renderClusterSelectorItem = ({ - title, option, filterType, + title, option, filterTypes, }) => { if (!cellSets.accessible) { return ( @@ -113,7 +113,7 @@ const DiffExprCompute = (props) => { { renderClusterSelectorItem({ title: 'Compare cell set:', option: 'cellSet', - filterType: 'cellSets', + filterTypes: ['cellSets', 'CLM'], }) } {renderClusterSelectorItem({ title: 'and cell set:', option: 'compareWith', - filterType: 'cellSets', + filterTypes: ['cellSets', 'CLM'], })} {renderClusterSelectorItem({ title: 'within sample/group:', option: 'basis', - filterType: 'metadataCategorical', + filterTypes: ['metadataCategorical', 'CLMPerSample'], })} ) : ( @@ -252,19 +252,19 @@ const DiffExprCompute = (props) => { {renderClusterSelectorItem({ title: 'Compare cell set:', option: 'basis', - filterType: 'cellSets', + filterTypes: ['cellSets', 'CLM'], })} {renderClusterSelectorItem({ title: 'between sample/group:', option: 'cellSet', - filterType: 'metadataCategorical', + filterTypes: ['metadataCategorical', 'CLMPerSample'], })} {renderClusterSelectorItem({ title: 'and sample/group:', option: 'compareWith', - filterType: 'metadataCategorical', + filterTypes: ['metadataCategorical', 'CLMPerSample'], })} )} @@ -278,7 +278,7 @@ const DiffExprCompute = (props) => { canRunDiffExprResults.FALSE, canRunDiffExprResults.INSUFFCIENT_CELLS_ERROR, ].includes(canRunDiffExpr()) - || (needPValues && canRunDiffExpr() !== canRunDiffExprResults.TRUE)} + || (needPValues && canRunDiffExpr() !== canRunDiffExprResults.TRUE)} onClick={() => onCompute()} > Compute diff --git a/src/components/data-exploration/differential-expression-tool/DiffExprSelect.jsx b/src/components/data-exploration/differential-expression-tool/DiffExprSelect.jsx index fa82178bb8..c842b4bceb 100644 --- a/src/components/data-exploration/differential-expression-tool/DiffExprSelect.jsx +++ b/src/components/data-exploration/differential-expression-tool/DiffExprSelect.jsx @@ -9,12 +9,15 @@ const { Option, OptGroup } = Select; const DiffExprSelect = (props) => { const { - title, option, filterType, onSelectCluster, selectedComparison, cellSets, value + title, option, filterTypes, onSelectCluster, selectedComparison, cellSets, value } = props; // Depending on the cell set type specified, set the default name - const placeholder = filterType === 'metadataCategorical' ? 'sample/group' : 'cell set'; + const placeholder = filterTypes.includes('metadataCategorical') ? 'sample/group' : 'cell set'; const { hierarchy, properties } = cellSets; - const tree = composeTree(hierarchy, properties, filterType); + const tree = composeTree(hierarchy, properties, filterTypes); + + console.log(filterTypes, 'treeDebug'); + console.log(tree); const renderChildren = (rootKey, children) => { if (!children || children.length === 0) { return (<>); } @@ -96,7 +99,7 @@ DiffExprSelect.propTypes = { selectedComparison: PropTypes.object.isRequired, cellSets: PropTypes.object.isRequired, option: PropTypes.string.isRequired, - filterType: PropTypes.string.isRequired, + filterTypes: PropTypes.array.isRequired, onSelectCluster: PropTypes.func.isRequired, }; export default DiffExprSelect; diff --git a/src/components/data-management/metadata/AddMetadataButton.jsx b/src/components/data-management/metadata/AddMetadataButton.jsx index afa588795c..90ac7eb4d7 100644 --- a/src/components/data-management/metadata/AddMetadataButton.jsx +++ b/src/components/data-management/metadata/AddMetadataButton.jsx @@ -103,7 +103,7 @@ const AddMetadataButton = ({ samplesTableRef }) => { ), - disabled: true, + disabled: false, onClick: () => { setCellLevelUploadVisible(true); }, diff --git a/src/utils/cellSets/composeTree.js b/src/utils/cellSets/composeTree.js index fed373fbd9..099e4df0e6 100644 --- a/src/utils/cellSets/composeTree.js +++ b/src/utils/cellSets/composeTree.js @@ -6,13 +6,13 @@ * all cell sets, including metadata. * */ -const composeTree = (hierarchy, properties, filterType = null) => { - const composeTreeRecursive = (data, type) => { +const composeTree = (hierarchy, properties, filterTypes = null) => { + const composeTreeRecursive = (data, types) => { if (!data) { return; } return data.filter( - (root) => (!type || properties[root.key].type === type), + (root) => (!types || types.includes(properties[root.key].type)), ).map( (node) => { // eslint-disable-next-line no-unused-vars @@ -27,7 +27,7 @@ const composeTree = (hierarchy, properties, filterType = null) => { }, ); }; - return composeTreeRecursive(hierarchy, filterType); + return composeTreeRecursive(hierarchy, filterTypes); }; export default composeTree; From a0fc7f7c88beb3e6d2fb029671546faa5a2d5293 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Wed, 15 Nov 2023 09:26:21 -0300 Subject: [PATCH 2/7] Switch to using filterTypes for batch diff expr table Signed-off-by: cosa65 --- .../batch-differential-expression/index.jsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pages/experiments/[experimentId]/plots-and-tables/batch-differential-expression/index.jsx b/src/pages/experiments/[experimentId]/plots-and-tables/batch-differential-expression/index.jsx index bacf86a57c..6782771453 100644 --- a/src/pages/experiments/[experimentId]/plots-and-tables/batch-differential-expression/index.jsx +++ b/src/pages/experiments/[experimentId]/plots-and-tables/batch-differential-expression/index.jsx @@ -196,7 +196,7 @@ const BatchDiffExpression = (props) => { changeComparison({ cellSet })} selectedComparison={{ cellSet: comparison.cellSet }} value={comparison.cellSet} @@ -205,7 +205,7 @@ const BatchDiffExpression = (props) => { changeComparison({ compareWith: cellSet })} selectedComparison={{ cellSet: comparison.cellSet }} value={comparison.compareWith} @@ -231,7 +231,7 @@ const BatchDiffExpression = (props) => { changeComparison({ cellSet })} selectedComparison={{ cellSet: comparison.cellSet }} value={comparison.cellSet} @@ -240,7 +240,7 @@ const BatchDiffExpression = (props) => { changeComparison({ compareWith: cellSet })} selectedComparison={{ cellSet: comparison.cellSet }} value={comparison.compareWith} From 45c361714fa97dd5266de461a069955fe1f9a6e0 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Wed, 15 Nov 2023 10:03:30 -0300 Subject: [PATCH 3/7] Remove log Signed-off-by: cosa65 --- .../differential-expression-tool/DiffExprSelect.jsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/components/data-exploration/differential-expression-tool/DiffExprSelect.jsx b/src/components/data-exploration/differential-expression-tool/DiffExprSelect.jsx index c842b4bceb..356c212a2f 100644 --- a/src/components/data-exploration/differential-expression-tool/DiffExprSelect.jsx +++ b/src/components/data-exploration/differential-expression-tool/DiffExprSelect.jsx @@ -16,9 +16,6 @@ const DiffExprSelect = (props) => { const { hierarchy, properties } = cellSets; const tree = composeTree(hierarchy, properties, filterTypes); - console.log(filterTypes, 'treeDebug'); - console.log(tree); - const renderChildren = (rootKey, children) => { if (!children || children.length === 0) { return (<>); } From 567b1e8406f82d6db97d5fc76f05bc66adcfc441 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Thu, 16 Nov 2023 08:47:55 -0300 Subject: [PATCH 4/7] Pass CLM and CLMPerSample to corresponding selects Signed-off-by: cosa65 --- .../batch-differential-expression/index.jsx | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/pages/experiments/[experimentId]/plots-and-tables/batch-differential-expression/index.jsx b/src/pages/experiments/[experimentId]/plots-and-tables/batch-differential-expression/index.jsx index 6782771453..36783170f0 100644 --- a/src/pages/experiments/[experimentId]/plots-and-tables/batch-differential-expression/index.jsx +++ b/src/pages/experiments/[experimentId]/plots-and-tables/batch-differential-expression/index.jsx @@ -53,8 +53,12 @@ const BatchDiffExpression = (props) => { const cellSets = useSelector(getCellSets()); const { properties, hierarchy } = cellSets; const experimentName = useSelector((state) => state.experimentSettings.info.experimentName); + const cellSetNodes = useSelector(getCellSetsHierarchyByType('cellSets')); + const clmNodes = useSelector(getCellSetsHierarchyByType('CLM')); + const metadataCellSetNodes = useSelector(getCellSetsHierarchyByType('metadataCategorical')); + const clmPerSampleNodes = useSelector(getCellSetsHierarchyByType('CLMPerSample')); const [dataLoading, setDataLoading] = useState(); @@ -65,6 +69,16 @@ const BatchDiffExpression = (props) => { const [sample] = useSelector(getCellSetsHierarchyByKeys(['sample'])); + const cellDefinedNodes = useMemo( + () => [...cellSetNodes, ...clmNodes], + [cellSetNodes, clmNodes], + ); + + const sampleDefinedNodes = useMemo( + () => [...metadataCellSetNodes, ...clmPerSampleNodes], + [metadataCellSetNodes, clmPerSampleNodes], + ); + const isDatasetUnisample = useMemo(() => sample?.children.length === 1, [sample]); useEffect(() => { dispatch(loadCellSets(experimentId)); @@ -182,7 +196,7 @@ const BatchDiffExpression = (props) => { onChange={(value) => changeComparison({ basis: value })} value={comparison.basis} style={{ width: '40%' }} - options={getSelectOptions(cellSetNodes)} + options={getSelectOptions(cellDefinedNodes)} />
@@ -218,7 +232,7 @@ const BatchDiffExpression = (props) => { onChange={(value) => changeComparison({ basis: value })} value={comparison.basis} style={{ width: '33.5%' }} - options={getSelectOptions(cellSetNodes)} + options={getSelectOptions(cellDefinedNodes)} /> ); @@ -253,7 +267,7 @@ const BatchDiffExpression = (props) => { onChange={(value) => changeComparison({ basis: value })} value={comparison.basis} style={{ width: '34%' }} - options={getSelectOptions(metadataCellSetNodes)} + options={getSelectOptions(sampleDefinedNodes)} /> ); From 62ad625e7f0636659e8a922737eda903af3c8b29 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Thu, 16 Nov 2023 09:51:02 -0300 Subject: [PATCH 5/7] Add mock cell level cell sets to the batch differential tests Signed-off-by: cosa65 --- src/__test__/data/cell_level_cell_sets.json | 66 +++++++++++++++++++ .../index.test.jsx | 18 ++++- 2 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 src/__test__/data/cell_level_cell_sets.json diff --git a/src/__test__/data/cell_level_cell_sets.json b/src/__test__/data/cell_level_cell_sets.json new file mode 100644 index 0000000000..961f66fc84 --- /dev/null +++ b/src/__test__/data/cell_level_cell_sets.json @@ -0,0 +1,66 @@ +[{ + "key": "clm-0", + "name": "Some Cell Level Track", + "rootNode": true, + "children": [ + { + "key": "clm-0-0", + "name": "Some cell level zero", + "color": "#8c564b", + "cellIds": [1, 21, 51, 86] + }, + { + "key": "clm-0-1", + "name": "Some cell level one", + "color": "#d62728", + "cellIds": [11, 42, 96] + } + ], + "type": "CLM" +}, +{ + "key": "clm-1", + "name": "Another Cell Level Track", + "rootNode": true, + "children": [ + { + "key": "clm-1-0", + "name": "Another cell level zero", + "color": "#8c564b", + "cellIds": [2, 22, 52, 87] + }, + { + "key": "clm-1-1", + "name": "Another cell level one", + "color": "#d62728", + "cellIds": [15, 47, 49] + } + ], + "type": "CLM" +}, +{ + "key": "sample-clm-0", + "name": "Sample Cell Level Track", + "rootNode": true, + "children": [ + { + "key": "sample-clm-0-0", + "name": "Sample cell level zero", + "color": "#8c564b", + "cellIds": [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, + 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, + 21, 22, 23, 24, 25, 26, 27, 28, 29 , 30, + 31, 32, 33, 34, 35, 36, 37, 38, 39, 40] + }, + { + "key": "sample-clm-0-1", + "name": "Sample cell level one", + "color": "#d62728", + "cellIds": [41, 42, 43, 44, 45, 46, 47, 48, 49, 50, + 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, + 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, + 71, 72, 73, 74, 75] + } + ], + "type": "CLMPerSample" +}] \ No newline at end of file diff --git a/src/__test__/pages/experiments/[experimentId]/plots-and-tables/batch-differential-expression/index.test.jsx b/src/__test__/pages/experiments/[experimentId]/plots-and-tables/batch-differential-expression/index.test.jsx index 1b6e4ffc21..0cdc2989d0 100644 --- a/src/__test__/pages/experiments/[experimentId]/plots-and-tables/batch-differential-expression/index.test.jsx +++ b/src/__test__/pages/experiments/[experimentId]/plots-and-tables/batch-differential-expression/index.test.jsx @@ -11,18 +11,30 @@ import userEvent from '@testing-library/user-event'; import fetchMock, { enableFetchMocks } from 'jest-fetch-mock'; import { makeStore } from 'redux/store'; -import mockAPI, { generateDefaultMockAPIResponses } from '__test__/test-utils/mockAPI'; +import mockAPI, { generateDefaultMockAPIResponses, promiseResponse } from '__test__/test-utils/mockAPI'; import * as getBatchDiffExpr from 'utils/extraActionCreators/differentialExpression/getBatchDiffExpr'; import * as checkCanRunDiffExprModule from 'utils/extraActionCreators/differentialExpression/checkCanRunDiffExpr'; import mockLoader from 'components/Loader'; +import cellSetsData from '__test__/data/cell_sets.json'; +import cellLevelCellSets from '__test__/data/cell_level_cell_sets.json'; + jest.spyOn(checkCanRunDiffExprModule, 'default').mockImplementation(() => 'TRUE'); jest.mock('utils/extraActionCreators/differentialExpression/getBatchDiffExpr'); jest.mock('components/Loader', () => jest.fn(() =>
Mock Loader
)); describe('Batch differential expression tests ', () => { let storeState = null; - const mockApiResponses = _.merge(generateDefaultMockAPIResponses(fake.EXPERIMENT_ID)); + + const customCellSetsData = _.cloneDeep(cellSetsData); + // Add the cell level cell sets + customCellSetsData.cellSets.push(...cellLevelCellSets); + + const mockApiResponses = { + ...generateDefaultMockAPIResponses(fake.EXPERIMENT_ID), + [`experiments/${fake.EXPERIMENT_ID}/cellSets$`]: () => promiseResponse(JSON.stringify(customCellSetsData)), + }; + let getBatchDiffExprSpy; beforeEach(async () => { @@ -35,6 +47,7 @@ describe('Batch differential expression tests ', () => { storeState = makeStore(); getBatchDiffExprSpy = jest.spyOn(getBatchDiffExpr, 'default'); }); + const secondOptionText = 'Compare between two selected samples/groups in a cell set for all cell sets'; const renderPage = async () => { await act(async () => render( @@ -107,6 +120,7 @@ describe('Batch differential expression tests ', () => { ['louvain-0', 'louvain-1', 'louvain-2', 'louvain-3', 'louvain-4', 'louvain-5', 'louvain-6', 'louvain-7', 'louvain-8', 'louvain-9', 'louvain-10', 'louvain-11', 'louvain-12', 'louvain-13']); }); }); + it('shows the Loader while fetching data', async () => { getBatchDiffExpr.mockImplementation(() => new Promise((resolve) => { setTimeout(() => { From 062bbafc94078e4b295cb2128872acefd6d60319 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Thu, 16 Nov 2023 11:31:54 -0300 Subject: [PATCH 6/7] Improve test Signed-off-by: cosa65 --- .../index.test.jsx | 64 +++++++++++++++++-- 1 file changed, 58 insertions(+), 6 deletions(-) diff --git a/src/__test__/pages/experiments/[experimentId]/plots-and-tables/batch-differential-expression/index.test.jsx b/src/__test__/pages/experiments/[experimentId]/plots-and-tables/batch-differential-expression/index.test.jsx index 0cdc2989d0..f6d2221d1f 100644 --- a/src/__test__/pages/experiments/[experimentId]/plots-and-tables/batch-differential-expression/index.test.jsx +++ b/src/__test__/pages/experiments/[experimentId]/plots-and-tables/batch-differential-expression/index.test.jsx @@ -65,23 +65,75 @@ describe('Batch differential expression tests ', () => { it('Shows correct input fields for each comparison option', async () => { await renderPage(); - const compareForCellSetsRadio = screen.getByLabelText(secondOptionText); - const compareForSamplesRadio = screen.getByLabelText(/Compare two cell sets for all samples\/groups/i); + const compareBetweenSamplesRadio = screen.getByLabelText(secondOptionText); + const compareBetweenCellSetsRadio = screen.getByLabelText(/Compare two cell sets for all samples\/groups/i); expect(screen.getByText(/Select the cell sets for which marker genes are to be computed in batch:/i)).toBeInTheDocument(); expect(screen.getByText('Select a cell set...')).toBeInTheDocument(); - // Check compareForCellSetsRadio - await act(() => userEvent.click(compareForCellSetsRadio)); + await act(() => userEvent.click(screen.getByText('Select a cell set...'))); + + const cellBasedClasses = [ + 'Fake louvain clusters', + 'Custom cell sets', + 'Some Cell Level Track', + 'Another Cell Level Track' + ]; + + const sampleBasedClasses = [ + 'Samples', + 'Track_1', + 'Sample Cell Level Track' + ]; + + const sampleBasedSets = [ + 'KO', 'WT1', 'WT2', 'KMeta', 'WMetaT', 'Sample cell level zero', 'Sample cell level one', + ]; + + // Shows the correct cell classes as options + cellBasedClasses.forEach((text) => { + expect(screen.getByText(text)).toBeInTheDocument(); + }) + + // Doesn't show the samples or sample-based metadata cell classes as options + sampleBasedClasses.forEach((text) => { + expect(screen.queryByText(text)).not.toBeInTheDocument(); + }) + + // Check compareBetweenSamplesRadio + await act(() => userEvent.click(compareBetweenSamplesRadio)); expect(screen.getByText(/Select the comparison sample\/groups for which batch/i)).toBeInTheDocument(); expect(screen.getByText(/In batch for each cell set in:/i)).toBeInTheDocument(); expect(screen.getByText(/Select a cell set.../i)).toBeInTheDocument(); - // Check compareForSamplesRadio - await act(() => userEvent.click(compareForSamplesRadio)); + expect(screen.getAllByText('Select a sample/group...')).toHaveLength(2); + screen.getAllByText('Select a sample/group...').forEach((match) => { + expect(match).toBeVisible(); + }); + + await act(() => userEvent.click(screen.getAllByText('Select a sample/group...')[0])); + + // Shows the sample options and their classes + [...sampleBasedSets, ...sampleBasedClasses].forEach((text) => { + expect(screen.getByText(text)).toBeInTheDocument(); + }); + + // Check compareBetweenCellSetsRadio + await act(() => userEvent.click(compareBetweenCellSetsRadio)); expect(screen.getByText(/Select the comparison cell sets for which batch/i)).toBeInTheDocument(); expect(screen.getByText(/In batch for each sample\/group in:/i)).toBeInTheDocument(); expect(screen.getByText(/Select samples or metadata.../i)).toBeInTheDocument(); + + await act(() => userEvent.click(screen.getAllByText('Select a cell set...')[0])); + + + const someLouvainCellSets = Array.from({ length: 11 }, (x, index) => `Cluster ${index}`); + // Shows only some of the louvain options and not the others + // Because it is a virtual list which only renders the options that enter the + // options that fit in the display + [...someLouvainCellSets, 'fake louvain clusters'].forEach((text) => { + expect(screen.getByText(text)).toBeInTheDocument(); + }); }); it('sending a request should work', async () => { From 725d649ad6a3e0d26a8d81bdfcac684431b8aae2 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Thu, 16 Nov 2023 11:46:08 -0300 Subject: [PATCH 7/7] Cleanup Signed-off-by: cosa65 --- src/components/data-management/metadata/AddMetadataButton.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/data-management/metadata/AddMetadataButton.jsx b/src/components/data-management/metadata/AddMetadataButton.jsx index 342363b963..50895afec1 100644 --- a/src/components/data-management/metadata/AddMetadataButton.jsx +++ b/src/components/data-management/metadata/AddMetadataButton.jsx @@ -1,7 +1,7 @@ /* eslint-disable no-param-reassign */ import React, { useState } from 'react'; import { - Dropdown, Button, Tooltip, + Dropdown, Button, } from 'antd'; import PropTypes from 'prop-types';