Skip to content

NickAkhmetov/SCFind Genes Query #3737

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

Merged
merged 18 commits into from
May 15, 2025
Merged

Conversation

NickAkhmetov
Copy link
Collaborator

Summary

This PR enables the SCFind method for gene queries on the molecular and biological queries page and applies cleanup for previous work related to cell types based on feedback in that branch.

Design Documentation/Original Tickets

Testing

Manually tested various gene queries, regression tested cell types query to make sure that didn't break.

Screenshots/Video

image
image
image

Checklist

  • Code follows the project's coding standards
    • Lint checks pass locally
    • New CHANGELOG-your-feature-name-here.md is present in the root directory, describing the change(s) in full sentences.
  • Unit tests covering the new feature have been added
  • All existing tests pass
  • Any relevant documentation in JIRA/Confluence has been updated to reflect the new feature
  • Any new functionalities have appropriate analytics functionalities added

Additional Notes

  • The datasets overview graph which compares the matched datasets to the set of indexed/all datasets in HuBMAP will be implemented in a subsequent PR.
  • The Gene Pathways integration with UBKG will also be implemented in a subsequent PR.

@NickAkhmetov NickAkhmetov requested a review from Copilot May 8, 2025 15:38
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enables gene queries using the scFind method and cleans up legacy code related to cell types. Key changes include updating query method defaults in the molecular data query form, refactoring autocomplete logic, and introducing new chart components for gene expression and cell type distribution tailored for scFind data.

Reviewed Changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated no comments.

Show a summary per file
File Description
MolecularDataQueryForm.tsx Updated the default queryMethod to 'scFind' for gene queries and adjusted query method resets.
QueryMethod.tsx Added scFind as an active gene query option and removed a duplicate entry.
AutocompleteEntity/hooks.ts Updated the condition for handling scFind autocomplete to also check the targetEntity value.
SCFindGeneCharts.tsx & CellTypesChart.tsx Introduced new chart components for gene expression level and cell type distributions.
Various API hooks (useFindDatasetForGenes.ts, useCellTypeExpression.ts, useCLIDToLabel.ts) Updated request key construction and parameter handling to support scFind gene queries.
DatasetsOverview & FractionGraph/Related Files Cleaned up outdated components and adjusted layout/documentation components accordingly.
Comments suppressed due to low confidence (4)

context/app/static/js/components/cells/MolecularDataQueryForm/MolecularDataQueryForm.tsx:24

  • Changing the default queryMethod to 'scFind' for gene queries impacts the user experience; please ensure that this update aligns with the intended behavior and communication with stakeholders.
queryMethod: 'scFind',

context/app/static/js/components/cells/MolecularDataQueryForm/AutocompleteEntity/hooks.ts:24

  • The additional check on targetEntity being 'cell-type' may limit autocomplete behavior when using scFind; confirm that this condition is intentional and that gene queries using scFind do not require autocomplete.
if (queryMethod === 'scFind' && targetEntity === 'cell-type') {

context/app/static/js/api/scfind/useCellTypeExpression.ts:30

  • Usage of String.prototype.replaceAll requires a supported environment; verify that target browsers or polyfills are in place if older environments must be supported.
cell_type: `${datasetName.replaceAll('.', '-')}.${datasetName}`,

context/app/static/js/api/scfind/useCLIDToLabel.ts:20

  • Switching the key to 'CLID2CellType' might affect consumers of this hook; please ensure all dependent components are updated accordingly.
return createScFindKey(scFindEndpoint, 'CLID2CellType', {

Comment on lines +38 to +39
// TODO: Update with correct response type once the API is fixed
return useSWR<GeneExpressionBinsResponse, unknown, GeneExpressionBinKey>(key, (url) => fetcher({ url }));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Still pending resolution of this issue; however, since the cell type expression bins endpoint works, it's okay.

@NickAkhmetov NickAkhmetov marked this pull request as ready for review May 8, 2025 15:55
Copy link
Collaborator

@austenem austenem left a comment

Choose a reason for hiding this comment

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

Great work! Just a few minor comments.

</Box>
)}
</TooltipInPortal>
)}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This could potentially be split into sub-components, particularly the tooltip and legend.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed, this can be more modular. Since this was already largely an effort to break down the Cell Type Distribution Chart component into smaller subcomponents, I'll break it down further.

{decimal.format(totalCells)}
</Typography>
);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Optional, but these column components are almost identical - could add an additional prop here instead to differentiate between the final return clauses.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's tricky to add additional props to these column/cell components since they have to conform to CellContentProps; however, I should be able to reduce repetition via composition instead.

onMouseLeave: MouseEventHandler<HTMLOrSVGElement>;
onFocusCapture: FocusEventHandler<HTMLOrSVGElement>;
onBlurCapture: FocusEventHandler<HTMLOrSVGElement>;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This return type could be separated out into an interface to avoid the repetition in the function body

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good call 👍🏻

@NickAkhmetov NickAkhmetov requested a review from austenem May 14, 2025 15:21
Copy link
Collaborator

@austenem austenem left a comment

Choose a reason for hiding this comment

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

Looks great! 🚀

@NickAkhmetov NickAkhmetov merged commit 64b9d60 into main May 15, 2025
8 checks passed
@NickAkhmetov NickAkhmetov deleted the nickakhmetov/scfind-genes-query branch May 15, 2025 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants