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

Disable apps based on study metadata #732

Merged
merged 17 commits into from
Dec 20, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
353 changes: 190 additions & 163 deletions packages/libs/eda/src/lib/core/components/computations/StartPage.tsx

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { GeoConfig } from '../../types/geoConfig';
import { Computation, ComputationAppOverview } from '../../types/visualization';
import { Filter, StudyEntity } from '../..';
import { VisualizationPlugin } from '../visualizations/VisualizationPlugin';
import { IsEnabledInPickerParams } from '../visualizations/VisualizationTypes';

export interface ComputationProps {
analysisState: AnalysisState;
Expand Down Expand Up @@ -50,4 +51,8 @@ export interface ComputationPlugin {
rootEntity: StudyEntity
) => Record<string, unknown> | undefined;
isConfigurationComplete: (configuration: unknown) => boolean;
/** Function used to determine if visualization is compatible with study */
isEnabledInPicker?: (props: IsEnabledInPickerParams) => boolean;
/** Human-readable study requirements for this computation */
studyRequirements?: string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import { ComputationStepContainer } from '../ComputationStepContainer';
import './Plugins.scss';
import { makeClassNameHelper } from '@veupathdb/wdk-client/lib/Utils/ComponentUtils';
import { VariableCollectionSelectList } from '../../variableSelectors/VariableCollectionSingleSelect';
import { IsEnabledInPickerParams } from '../../visualizations/VisualizationTypes';
import { entityTreeToArray } from '../../../utils/study-metadata';

const cx = makeClassNameHelper('AppStepConfigurationContainer');

Expand Down Expand Up @@ -95,6 +97,9 @@ export const plugin: ComputationPlugin = {
hideShowMissingnessToggle: true,
}),
},
isEnabledInPicker: isEnabledInPicker,
studyRequirements:
'These visualizations are only available for studies with compatible assay data.',
};

function AbundanceConfigDescriptionComponent({
Expand Down Expand Up @@ -195,3 +200,19 @@ export function AbundanceConfiguration(props: ComputationConfigProps) {
</ComputationStepContainer>
);
}

// The abundance app's only requirement for the study is that the study
// contains at least one collection.
function isEnabledInPicker({
studyMetadata,
}: IsEnabledInPickerParams): boolean {
if (!studyMetadata) return false;

const entities = entityTreeToArray(studyMetadata.rootEntity);
// Ensure there are collections in this study. Otherwise, disable app
const studyHasCollections = entities.some(
(entity) => !!entity.collections?.length
);
Copy link
Member

Choose a reason for hiding this comment

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

How important is the type predicate assertion here? If we were filtering and then doing something with the result (especially if an array result of StudyEntities), it would be useful. But I'm not sure the extra complication wins us anything here because the some just returns true/false.

Copy link
Member Author

Choose a reason for hiding this comment

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

True. Simplified in the latest commit!


return studyHasCollections;
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import { ComputationStepContainer } from '../ComputationStepContainer';
import './Plugins.scss';
import { makeClassNameHelper } from '@veupathdb/wdk-client/lib/Utils/ComponentUtils';
import { VariableCollectionSelectList } from '../../variableSelectors/VariableCollectionSingleSelect';
import { IsEnabledInPickerParams } from '../../visualizations/VisualizationTypes';
import { entityTreeToArray } from '../../../utils/study-metadata';

const cx = makeClassNameHelper('AppStepConfigurationContainer');

Expand Down Expand Up @@ -60,6 +62,9 @@ export const plugin: ComputationPlugin = {
hideShowMissingnessToggle: true,
}),
},
isEnabledInPicker: isEnabledInPicker,
studyRequirements:
'These visualizations are only available for studies with compatible assay data.',
};

function AlphaDivConfigDescriptionComponent({
Expand Down Expand Up @@ -159,3 +164,19 @@ export function AlphaDivConfiguration(props: ComputationConfigProps) {
</ComputationStepContainer>
);
}

// Alpha div's only requirement of the study is that
// the study contains at least one collection.
function isEnabledInPicker({
studyMetadata,
}: IsEnabledInPickerParams): boolean {
if (!studyMetadata) return false;
const entities = entityTreeToArray(studyMetadata.rootEntity);

// Ensure there are collections in this study. Otherwise, disable app
const studyHasCollections = entities.some(
(entity) => !!entity.collections?.length
);

return studyHasCollections;
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import { ComputationStepContainer } from '../ComputationStepContainer';
import './Plugins.scss';
import { makeClassNameHelper } from '@veupathdb/wdk-client/lib/Utils/ComponentUtils';
import { VariableCollectionSelectList } from '../../variableSelectors/VariableCollectionSingleSelect';
import { IsEnabledInPickerParams } from '../../visualizations/VisualizationTypes';
import { entityTreeToArray } from '../../../utils/study-metadata';

const cx = makeClassNameHelper('AppStepConfigurationContainer');

Expand Down Expand Up @@ -63,6 +65,9 @@ export const plugin: ComputationPlugin = {
})
.withSelectorIcon(ScatterBetadivSVG),
},
isEnabledInPicker: isEnabledInPicker,
studyRequirements:
'These visualizations are only available for studies with compatible assay data.',
};

function BetaDivConfigDescriptionComponent({
Expand Down Expand Up @@ -172,3 +177,19 @@ export function BetaDivConfiguration(props: ComputationConfigProps) {
</ComputationStepContainer>
);
}

// Beta div's only requirement of the study is that it contains
// at least one collection
function isEnabledInPicker({
studyMetadata,
}: IsEnabledInPickerParams): boolean {
if (!studyMetadata) return false;
const entities = entityTreeToArray(studyMetadata.rootEntity);

// Ensure there are collections in this study. Otherwise, disable app
const studyHasCollections = entities.some(
(entity) => !!entity.collections?.length
);

return studyHasCollections;
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import { bipartiteNetworkVisualization } from '../../visualizations/implementati
import { variableCollectionsAreUnique } from '../../../utils/visualization';
import PluginError from '../../visualizations/PluginError';
import { VariableCollectionSelectList } from '../../variableSelectors/VariableCollectionSingleSelect';
import { IsEnabledInPickerParams } from '../../visualizations/VisualizationTypes';
import { entityTreeToArray } from '../../../utils/study-metadata';

const cx = makeClassNameHelper('AppStepConfigurationContainer');

Expand Down Expand Up @@ -73,6 +75,9 @@ export const plugin: ComputationPlugin = {
},
}), // Must match name in data service and in visualization.tsx
},
isEnabledInPicker: isEnabledInPicker,
studyRequirements:
'These visualizations are only available for studies with metagenomic data.',
};

// Renders on the thumbnail page to give a summary of the app instance
Expand Down Expand Up @@ -198,3 +203,23 @@ export function CorrelationAssayAssayConfiguration(
</ComputationStepContainer>
);
}

// The correlation assay x assay app should only be available
// for studies with metagenomic data.
function isEnabledInPicker({
studyMetadata,
}: IsEnabledInPickerParams): boolean {
if (!studyMetadata) return false;

const entities = entityTreeToArray(studyMetadata.rootEntity);
// Ensure there are collections in this study. Otherwise, disable app
const studyHasCollections = entities.some(
(entity) => !!entity.collections?.length
);
if (!studyHasCollections) return false;

const hasMetagenomicData =
entities.filter((entity) => entity.id === 'OBI_0002623').length > 0; // OBI_0002623 = Metagenomic sequencing assay

Copy link
Member

Choose a reason for hiding this comment

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

Do we also need to check for non-zero length collections here?

Copy link
Member Author

Choose a reason for hiding this comment

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

you're right we should. I was making the assumption that if that entity existed, then it would have a collection. But just becasue that's generally true doesn't mean it will always be the case! I'll add a check

Copy link
Member

Choose a reason for hiding this comment

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

Also, an entities.find(...) !== undefined might be very very slightly marginally more efficient! (because it stops the search as soon as it finds something - but these arrays are tiny, so it will make very little difference)

Copy link
Member

Choose a reason for hiding this comment

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

Or I guess entities.some(...) works too!

Copy link
Member

Choose a reason for hiding this comment

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

It's still possible that the study with collections and a OBI_0002623 assay with no collections could get through, I think?

Maybe just this?

  const hasMetagenomicData = entities.some((entity) => entity.id === 'OBI_0002623' && !!entity.collections?.length);

Copy link
Member Author

Choose a reason for hiding this comment

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

ack you're right! All fixed now!

I was interested so I looked into the difference in performance between some and find. It seems some is ever so slightly more efficient, since find calls some. But you're right it wouldn't matter on an array of this size anyway.

return hasMetagenomicData;
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ import { makeClassNameHelper } from '@veupathdb/wdk-client/lib/Utils/ComponentUt
import { H6 } from '@veupathdb/coreui';
import { bipartiteNetworkVisualization } from '../../visualizations/implementations/BipartiteNetworkVisualization';
import { VariableCollectionSelectList } from '../../variableSelectors/VariableCollectionSingleSelect';
import { entityTreeToArray } from '../../../utils/study-metadata';
import { IsEnabledInPickerParams } from '../../visualizations/VisualizationTypes';
import { ancestorEntitiesForEntityId } from '../../../utils/data-element-constraints';

const cx = makeClassNameHelper('AppStepConfigurationContainer');

Expand Down Expand Up @@ -62,6 +65,9 @@ export const plugin: ComputationPlugin = {
},
}), // Must match name in data service and in visualization.tsx
},
isEnabledInPicker: isEnabledInPicker,
studyRequirements:
'These visualizations are only available for studies with compatible metadata.',
};

// Renders on the thumbnail page to give a summary of the app instance
Expand Down Expand Up @@ -155,3 +161,65 @@ export function CorrelationAssayMetadataConfiguration(
</ComputationStepContainer>
);
}

const ASSAY_ENTITIES = [
'OBI_0002623',
'EUPATH_0000809',
'EUPATH_0000813',
'EUPATH_0000812',
];
Copy link
Member

Choose a reason for hiding this comment

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

Is there a long term plan (e.g. ticket) to annotate entities appropriately to reduce the amount of hardcoded entity IDs? (If that's agreed to be a plan.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Great question. I just made a ticket for it but i think it could use some more discussion. There wasn't originally a long-term plan because i didn't think that i would end up needing these assay ids at all until i got into the implementation.

To completely remove the hardcoded ids we'd need to add the following annotations:

  1. To the study, add hasMetagenomicData. I think there's a similar hasMapData annotation?
  2. To each entity, add isAssay. Or i suppose it could be something like entityType = 'assay', if there are other entity types that might be useful to us.

Also open to other solutions!


// The correlation assay x metadata app is only available for studies
// with appropriate metadata. Specifically, the study
// must have at least one continuous metadata variable that is on a one-to-one path
// from the assay entity.
// See PR #74 in service-eda-compute for the matching logic on the backend.
function isEnabledInPicker({
studyMetadata,
}: IsEnabledInPickerParams): boolean {
if (!studyMetadata) return false;

const entities = entityTreeToArray(studyMetadata.rootEntity);
// Ensure there are collections in this study. Otherwise, disable app
const studyHasCollections = entities.some(
(entity) => !!entity.collections?.length
);
if (!studyHasCollections) return false;

// Check for appropriate metadata
// Step 1. Find the first assay node. Doesn't need to be any assay in particular just any mbio assay will do
const firstAssayEntityIndex = entities.findIndex((entity) =>
ASSAY_ENTITIES.includes(entity.id)
);
if (firstAssayEntityIndex === -1) return false;

// Step 2. Find all ancestor entites of the assayEntity that are on a one-to-one path with assayEntity.
// Step 2a. Grab ancestor entities.
const ancestorEntities = ancestorEntitiesForEntityId(
entities[firstAssayEntityIndex].id,
entities
).reverse(); // Reverse so that the ancestorEntities[0] is the assay and higher indices are further up the tree.

// Step 2b. Trim the ancestorEntities so that we only keep those that are on
// a 1:1 path. Once we find an ancestor that is many to one with its parent, we
// know we've hit the end of the 1:1 path.
const lastOneToOneAncestorIndex = ancestorEntities.findIndex(
(entity) => entity.isManyToOneWithParent
);
const oneToOneAncestors = ancestorEntities.slice(
1, // removing the assay itself
Copy link
Member

Choose a reason for hiding this comment

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

So we assume there are no continuous 'metadata variables' on the same entity as the assay entity?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right. That assay entity should only have assay data.

lastOneToOneAncestorIndex + 1
);

// Step 3. Check if there are any continuous variables in the filtered entities
const hasContinuousVariable = oneToOneAncestors
.flatMap((entity) => entity.variables)
.some(
(variable) =>
'dataShape' in variable &&
variable.dataShape === 'continuous' &&
(variable.type === 'number' || variable.type === 'integer') // Support for dates coming soon! Can remove this line once the backend is ready.
);

return hasContinuousVariable;
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ import {
getBinRanges,
} from '../../../../map/analysis/utils/defaultOverlayConfig';
import { VariableCollectionSelectList } from '../../variableSelectors/VariableCollectionSingleSelect';
import { IsEnabledInPickerParams } from '../../visualizations/VisualizationTypes';
import { entityTreeToArray } from '../../../utils/study-metadata';

const cx = makeClassNameHelper('AppStepConfigurationContainer');

Expand Down Expand Up @@ -126,6 +128,9 @@ export const plugin: ComputationPlugin = {
},
}), // Must match name in data service and in visualization.tsx
},
isEnabledInPicker: isEnabledInPicker,
studyRequirements:
'These visualizations are only available for studies with compatible assay data.',
};

function DifferentialAbundanceConfigDescriptionComponent({
Expand Down Expand Up @@ -462,3 +467,19 @@ function assertConfigWithComparator(
);
}
}

// Differential abundance requires that the study
// has at least one collection.
function isEnabledInPicker({
studyMetadata,
}: IsEnabledInPickerParams): boolean {
if (!studyMetadata) return false;

const entities = entityTreeToArray(studyMetadata.rootEntity);
// Ensure there are collections in this study. Otherwise, disable app
const studyHasCollections = entities.some(
(entity) => !!entity.collections?.length
);

return studyHasCollections;
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,14 @@
font-weight: 300;
}
}
// Add disabled effects for the app rows
&-AppPicker {
&__disabled {
cursor: not-allowed;
border-radius: 20px;
color: #a6a6a6;
}
}

// add hover effect for thumbnail icons
&-ConfiguredVisualization:hover &-ConfiguredVisualizationActions {
Expand Down
2 changes: 1 addition & 1 deletion packages/libs/eda/src/lib/core/utils/study-metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
} from '../types/variable';
import { preorder } from '@veupathdb/wdk-client/lib/Utils/TreeUtils';

export function entityTreeToArray(rootEntity: StudyEntity) {
export function entityTreeToArray(rootEntity: StudyEntity): StudyEntity[] {
return Array.from(preorder(rootEntity, (e) => e.children ?? []));
}

Expand Down
Loading