Removed PdbAnnotation API dependency #1068

onursumer · 2018-04-04T16:38:54Z

What? Why?

Fix cBioPortal/cbioportal#3159.

Changes proposed in this pull request:

Now using Genome Nexus for PdbHeader data
Removed all the references, and files related to PdbAnnotationAPI

Checks

Is this PR adding logic based on one or more clinical attributes? If yes, please make sure validation for this attribute is also present in the data validation / data loading layers (in backend repo) and documented in File-Formats Clinical data section!
Follows 7 rules of great commit messages. For most PRs a single commit should suffice, in some cases multiple topical commits can be useful. During review it is ok to see tiny commits (e.g. Fix reviewer comments), but right before the code gets merged to master or rc branch, any such commits should be squashed since they are useless to the other developers. Definitely avoid merge commits, use rebase instead.
Follows the Airbnb React/JSX Style guide.
Make sure your commit messages end with a Signed-off-by string (this line
can be automatically added by git if you run the git-commit command with
the -s option)

onursumer · 2018-04-04T16:40:59Z

src/shared/cache/PdbPositionMappingCache.ts

-
-}
-
-export default class PdbPositionMappingCache extends LazyMobXCache<PdbUniprotResidueMapping, AlignmentIdAndUniportPos> {


We haven't been using this cache since merging #567, so removing until we re-implement it for the Genome Nexus endpoint data.

onursumer · 2018-04-04T16:47:47Z

src/shared/model/Pdb.ts

@@ -12,7 +12,7 @@ export interface IPdbPositionRange {

 export type PdbAlignmentIndex = {
    [pdbId: string]: {
-        [chainId: string]: PdbUniprotAlignment[]
+        [chainId: string]: Alignment[]


This should have been done by #567. Seems like it slipped away type-checking because in the PdbAnnotationAPI it was generated as any:

export type PdbUniprotAlignment = any;

adamabeshouse · 2018-04-04T18:51:49Z

src/pages/resultsView/mutation/MutationMapperStore.ts

    @cached get pdbPositionMappingCache()
    {
-        return new PdbPositionMappingCache();
+        // return new PdbPositionMappingCache();
+        return null;


do we not want to just delete this?

Ideally we should implement a new cache for the new structure, but for now it might be better to just remove it completely.

removed it completely for now, and added a new issue: cBioPortal/cbioportal#4071

alisman · 2018-04-17T21:00:48Z

src/shared/components/proteinChainPanel/column/OrganismColumnFormatter.tsx

@@ -12,12 +12,12 @@ export default class OrganismColumnFormatter {

        _.find(pdbHeader.compound, (mol:any)=>{
            if (_.indexOf(mol.chain, chainId.toLowerCase()) != -1 &&
-                pdbHeader.source[mol.mol_id] != null)
+                (pdbHeader.source as any)[mol.mol_id] != null)


curious why this isn't part of generated type?

Looks like we modeled the field source as a simple Map<String, Object> on the Genome Nexus side. Since we are getting this data from an external resource, we don't have the full model information. It is possible to define a partial model for the fields we use though.

onursumer · 2018-05-01T23:41:30Z

end-to-end-tests/specs/results.spec.js

+
+            it('populates PDB info properly', () => {
+                browser.click('[data-test=view3DStructure]');
+                browser.waitUntil(() => (browser.getText('[data-test=pdbChainInfoText]') !== "LOADING"), 10000);


@alisman @inodb not sure if this is the best way to wait until the actual text loads

@onursumer could use waitForExist but this works

@inodb The problem is that pdbChainInfoText always exists, but its text content changes from LOADING to a real value. And the actual content is generated by another generic component called TextExpander.

We could potentially render separate divs or spans for different states to be able to use waitForExist though.

…ortal#3159) Signed-off-by: Onur Sumer <s.onur.sumer@gmail.com>

inodb · 2018-05-29T20:15:44Z

end-to-end-tests/specs/results.spec.js

+
+            it('populates PDB info properly', () => {
+                browser.click('[data-test=view3DStructure]');
+                browser.waitUntil(() => (browser.getText('[data-test=pdbChainInfoText]') !== "LOADING"), 10000);


@onursumer could use waitForExist but this works

- Oncoprint tracks indicating profiled status in queried profiles (cBioPortal/cbioportal-frontend#1167) - Prevent alphabetical searching example queries list (cBioPortal/cbioportal-frontend#1177) - Fix missing modify query button for subset of virtual study (cBioPortal/cbioportal-frontend#1176) - Fix broken CN segments file (cBioPortal/cbioportal-frontend#1175) - Enable searching for germline in protein change column (cBioPortal/cbioportal-frontend#1152) - Open mycancergenome links in separate tab (cBioPortal/cbioportal-frontend#1188) - Fix copy button behavior (cBioPortal/cbioportal-frontend#1190) - Remove semicolon from custom news section (cBioPortal/cbioportal-frontend#1194) - Don't show invalid allele count values e.g. N/A, undefined, -1 in mutations table (cBioPortal/cbioportal-frontend#1165) - Remove dependency on pdb annotation service (use genomenexus.org instead) (cBioPortal/cbioportal-frontend#1068) - Fix genomic overview when it contains `.` in the filename (cBioPortal/cbioportal-frontend#1094)

onursumer requested review from alisman, adamabeshouse, jjgao and inodb April 4, 2018 16:38

onursumer commented Apr 4, 2018

View reviewed changes

adamabeshouse reviewed Apr 4, 2018

View reviewed changes

onursumer force-pushed the pdb-annotaion-cleanup branch 3 times, most recently from aa2f88a to 346e69a Compare April 4, 2018 19:34

adamabeshouse approved these changes Apr 4, 2018

View reviewed changes

onursumer force-pushed the pdb-annotaion-cleanup branch 2 times, most recently from c2a7f66 to 7a49276 Compare April 17, 2018 16:40

alisman reviewed Apr 17, 2018

View reviewed changes

onursumer force-pushed the pdb-annotaion-cleanup branch 3 times, most recently from 2fab493 to fef3106 Compare May 1, 2018 23:26

onursumer commented May 1, 2018

View reviewed changes

onursumer force-pushed the pdb-annotaion-cleanup branch 2 times, most recently from 8c550a0 to 973a164 Compare May 14, 2018 16:36

onursumer force-pushed the pdb-annotaion-cleanup branch from 973a164 to a5d789a Compare May 21, 2018 18:49

completely removed PdbAnnotation API dependency (fix cBioPortal/cbiop…

ccc5aa5

…ortal#3159) Signed-off-by: Onur Sumer <s.onur.sumer@gmail.com>

onursumer force-pushed the pdb-annotaion-cleanup branch from a5d789a to ccc5aa5 Compare May 29, 2018 17:51

inodb approved these changes May 29, 2018

View reviewed changes

Merge branch 'master' into pdb-annotaion-cleanup

0099720

inodb merged commit 672c597 into cBioPortal:master May 29, 2018

inodb mentioned this pull request May 29, 2018

update frontend version (v1.13.1-draft) cBioPortal/cbioportal#4284

Closed

onursumer deleted the pdb-annotaion-cleanup branch July 2, 2021 15:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Removed PdbAnnotation API dependency #1068

Removed PdbAnnotation API dependency #1068

onursumer commented Apr 4, 2018

onursumer Apr 4, 2018

onursumer Apr 4, 2018

adamabeshouse Apr 4, 2018

onursumer Apr 4, 2018

onursumer Apr 4, 2018

alisman Apr 17, 2018

onursumer Apr 18, 2018 •

edited

Loading

onursumer May 1, 2018

inodb May 29, 2018

onursumer May 29, 2018

inodb May 29, 2018


		}

		export default class PdbPositionMappingCache extends LazyMobXCache<PdbUniprotResidueMapping, AlignmentIdAndUniportPos> {

Removed PdbAnnotation API dependency #1068

Removed PdbAnnotation API dependency #1068

Conversation

onursumer commented Apr 4, 2018

What? Why?

Checks

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

onursumer Apr 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

onursumer Apr 18, 2018 •

edited

Loading