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

feat: gene info cards #298

Merged
merged 50 commits into from
Jul 26, 2022
Merged

feat: gene info cards #298

merged 50 commits into from
Jul 26, 2022

Conversation

sienacizdziel
Copy link
Contributor

@sienacizdziel sienacizdziel commented Jun 24, 2022

Reviewers

Functional:
frontend: @seve
backend in #323 : @MDunitz
Readability:


Depends on PRs:

Changes

  • create component for gene info pop-up
  • fetch gene ensembl ID from annoMatrix for more accurate search
  • call gene info API endpoint in data portal backend, which requests info from NCBI's E-Utilities API
  • edit gene component to include gene info button
  • use SDS styling and typing for TS
  • ensure smooth interactions between scatterplot and gene info pop-ups
  • e2e tests

Notes for Reviewers

For my intern project: gene info cards. This feature allows users to search gene information (e.g. gene description, summary, synonyms, etc) about a gene in question by clicking on an info circle next to a gene name. On info button click, a pop-up similar to the scatterplot feature appears. This works for genes in both quick gene and genesets.

@codecov
Copy link

codecov bot commented Jun 28, 2022

Codecov Report

Merging #298 (9496583) into main (9172e27) will increase coverage by 0.11%.
The diff coverage is 95.23%.

@@            Coverage Diff             @@
##             main     #298      +/-   ##
==========================================
+ Coverage   76.16%   76.28%   +0.11%     
==========================================
  Files          90       90              
  Lines        6670     6712      +42     
==========================================
+ Hits         5080     5120      +40     
- Misses       1590     1592       +2     
Flag Coverage Δ
frontend 76.28% <95.23%> (+0.11%) ⬆️
javascript 76.28% <95.23%> (+0.11%) ⬆️
smokeTest ?
unitTest 76.28% <95.23%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
server/common/rest.py 80.49% <90.90%> (+0.49%) ⬆️
server/app/api/v3.py 96.00% <100.00%> (+0.13%) ⬆️
server/common/config/server_config.py 86.99% <100.00%> (+0.35%) ⬆️
app/api/v3.py 96.00% <0.00%> (+0.13%) ⬆️
common/config/server_config.py 86.99% <0.00%> (+0.35%) ⬆️
common/rest.py 80.49% <0.00%> (+0.49%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us.

@sienacizdziel sienacizdziel changed the title Sienacizdziel/gene info feat: gene info cards Jun 28, 2022
@sienacizdziel sienacizdziel marked this pull request as ready for review July 13, 2022 20:18
@sienacizdziel sienacizdziel requested a review from seve July 13, 2022 20:19
Copy link
Contributor

@seve seve left a comment

Choose a reason for hiding this comment

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

WIP Review

client/src/actions/index.ts Outdated Show resolved Hide resolved
client/src/actions/index.ts Show resolved Hide resolved
client/src/annoMatrix/normalize.ts Show resolved Hide resolved
client/src/components/geneExpression/geneInfo/geneInfo.tsx Outdated Show resolved Hide resolved
client/src/components/geneExpression/geneInfo/geneInfo.tsx Outdated Show resolved Hide resolved
client/src/components/geneExpression/index.tsx Outdated Show resolved Hide resolved
Siena Cizdziel and others added 12 commits July 19, 2022 11:54
* adding in backend changes

* unit tests and status codes

* requested change

* requested change

Co-authored-by: Siena Cizdziel <[email protected]>
* feature id checks, reverting global rightsidebarwidth change, passing smoke tests

* chore: gene info icon and minor design changes (#320)

* icon changes

* requested design changes

Co-authored-by: Siena Cizdziel <[email protected]>

Co-authored-by: Siena Cizdziel <[email protected]>
* remove error checking for no feature id column

* functionality for datasets that don't have feature ids

* lint

* removing style changes

* hook up new api endpoint

* add gene param

Co-authored-by: Siena Cizdziel <[email protected]>
client/src/components/geneExpression/gene.tsx Outdated Show resolved Hide resolved
// if feature id column is available in var
if (annoMatrix.getMatrixColumns("var").includes(geneIdCol)) {
dfIds = await annoMatrix.fetch("var", geneIdCol);
console.log("id, success");
Copy link
Contributor

Choose a reason for hiding this comment

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

left in debug code?

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 removed the console.log, but the rest I think is still important for datasets that contain feature_id columns

console.error("Could not find feature IDs.");
}

setStatus("name, success");
Copy link
Contributor

Choose a reason for hiding this comment

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

why the change, just curious

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code is only for datasets that already have the "feature_id" column. Otherwise, we'll call the API with only gene name, since we don't have gene IDs built into the dataset (more of the code is implemented in the other PR that just recently got merged to this one)

client/src/components/leftSidebar/index.tsx Show resolved Hide resolved
client/src/reducers/controls.ts Outdated Show resolved Hide resolved
@seve seve self-requested a review July 26, 2022 23:41
@sienacizdziel sienacizdziel merged commit 8cf1341 into main Jul 26, 2022
@sienacizdziel sienacizdziel deleted the sienacizdziel/gene-info branch July 26, 2022 23:47
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