feat!: update tx segment element with updated cool-seq-tool structure changes #314

katiestahl · 2024-08-12T19:15:55Z

update naming of functions + variables
no more strand passed into tx segment element functions
changed GenomicData -> GenomicTxSegService
updated usage of Assayed/Categorical FusionElement in accordance with recent fusor change
updated tests - a lot of these are failing from weird asyncio issues, not sure if that was pre-existing or if I have stuff set up weirdly locally. I'm still looking into this, but from testing the app, everything looks like it's functioning correctly? Other than transcript segment element with using transcript/genomic coordinates. This appears to have broken either from the last PR or was already broken. This will be resolved when I finally get to resume feat: add transcript lookup for genes #273

Going to leave this in a draft for now, so I don't accidentally merge it before we have new CST and fusor releases with the changes that this PR implements

Close #272

… changes

github-actions · 2024-08-14T13:31:24Z

This PR is stale because it has been open 1 day(s) with no activity. Please review this PR.

katiestahl · 2024-08-22T18:10:09Z

server/tests/integration/test_utilities.py

            }
        },
        check_coords_response,
    )

    await check_response(
-        "/api/utilities/get_exon?chromosome=1",


we no longer support numbers for chromosomes, as far as I can tell. I think this is fine for now, especially since there's nowhere a user types in a chromosome (it's always auto-populated and unable to be edited)

katiestahl · 2024-08-22T18:11:13Z

server/tests/integration/test_validate.py

-    await check_validated_fusion_response(
-        async_client, ewsr1_fusion_fill_types, "EWSR1 fusion needing type inference"
-    )
+    # await check_validated_fusion_response(


I would like some opinions/eyes on this. I think because of the change I made in cancervariants/fusor#174, we can't do inference to the extent of before. Is this a problem? Is it okay to leave for now and make an issue for improvement later?

Looking at this now

How often does this happen in the UI where a user is going through the form and no type is provided?

I think we can save this for a new issue IMO. I think we'd just need to add a model_validator to AssayedFusion for this?

it's currently not possible to get to this state without a provided fusion type. The UI always adds the type in the request before calling the validation function

server/tests/conftest.py

…nce locations

jsstevenson · 2024-08-22T19:46:52Z

tests failing is a preexisting issue: #303

katiestahl · 2024-08-22T20:02:46Z

tests failing is a preexisting issue: #303

oh oop, should I remove that part of this PR?

jsstevenson · 2024-08-22T20:07:27Z

oh oop, should I remove that part of this PR?

Oh if you've fixed stuff I think it's fine to package in. I was just saying that some of those tests were already failing so it shouldn't hold back this PR if everything else seems ok

jsstevenson

👍

korikuzma · 2024-08-23T12:29:23Z

@jarbesfeld Are you able to review this?

jarbesfeld · 2024-08-23T12:37:01Z

@korikuzma Yes I can test UI as well

jarbesfeld

Nice work! See comment in conftest.py for a suggestion on how to improve one of the tests

jarbesfeld · 2024-08-23T12:43:45Z

server/tests/conftest.py

-        "exonStartOffset": 5,
+        "exonStartOffset": 72,
        "exonEnd": 6,
-        "exonEndOffset": -71,
-        "gene": tpm3_descriptor,
+        "exonEndOffset": -5,
+        "gene": tpm3_gene,


This example is technically correct, but it results in a Transcript Segment with only one nucleotide. I'd recommend in a different PR changing exonStart and exonEnd to be different values

I'm going to make a separate issue for this since this is technically correct

korikuzma · 2024-08-23T14:25:07Z

@katiestahl another issue that should be resolved is Transcript Segment Element needs either start or end to be valid. This is requiring start and end at the moment.

I'm also having more state issues. I fill out everything in structure, domain, reading frame, and look at summary. Once I go back to structure, I remove ending position but it still saves in the summary.

katiestahl · 2024-08-23T15:16:51Z

@katiestahl another issue that should be resolved is Transcript Segment Element needs either start or end to be valid. This is requiring start and end at the moment.
I'm also having more state issues. I fill out everything in structure, domain, reading frame, and look at summary. Once I go back to structure, I remove ending position but it still saves in the summary.

on it!

…ariants/fusion-curation into cool-seq-tool-tx-updates

…ill revisit at a later date

katiestahl · 2024-08-23T16:34:14Z

@katiestahl another issue that should be resolved is Transcript Segment Element needs either start or end to be valid. This is requiring start and end at the moment. I'm also having more state issues. I fill out everything in structure, domain, reading frame, and look at summary. Once I go back to structure, I remove ending position but it still saves in the summary.

on it!

so I am still trying to figure out why this is behaving like this. It looks like for the first TX Segment element, it's always invalid if it has a start (but it's fine if it has a start and end or only an end)

…ther a start or end now, regardless of position in the structure

katiestahl · 2024-08-23T16:47:54Z

client/src/components/Pages/Structure/Input/TxSegmentElementInput/TxSegmentElementInput.tsx

-      : index === 0
-      ? txEndingGenomic || endingExon
-      : txStartingGenomic || startingExon;
+    txEndingGenomic || endingExon || txStartingGenomic || startingExon;


please double check me on this change! From my understanding, based on the sequence location changes, a Tx segment element only requires either a start/end, regardless of position (it looks like previously, it had to have both unless it was the first item?)

Yes only one of start/end is required to create a TranscriptSegmentElement

korikuzma

The structure tab says the transcript segment element is valid (green check) but summary gives error. This appears to happen when invalid position is passed. I think we might want better error messages. This can be a new issue.

Screen.Recording.2024-08-26.at.07.06.37.mov

korikuzma · 2024-08-26T11:16:40Z

@katiestahl do you know why this one is failing?

Screen.Recording.2024-08-26.at.07.14.01.mov

korikuzma · 2024-08-26T11:18:22Z

client/src/components/Pages/Structure/Input/TxSegmentElementInput/TxSegmentElementInput.tsx

+          fieldValue={txChrom}
+          errorText={txChromText}
+          onChange={handleChromosomeChange}
+        />
        <Box mt="18px" width="125px">
          <StrandSwitch setStrand={setTxStrand} selectedStrand={txStrand} />


Strand can be removed from transcript segment element. Maybe templated sequence too (not sure how this one works)?

I'll remove the ability to change it for tx element. I think it might be fine to leave in the UI as a helpful visual tool to the user, though. what are your thoughts on that?

It looks like we still require the user to put in the strand for Templated Sequence, though. That element is pretty much entirely user-made, whereas tx makes a request to cool-seq-tool. If this is something cool-seq-tool can do now, we should def make a new issue for it!

pushed a commit where the ability to change it is removed, but it'll still show the strand to the user! I can also just remove it altogether- I just wasn't sure if it was a helpful visual aid or not

@katiestahl so it makes sense for transcript segment element when Genomic coordinates, gene is selected as input data since strand is automatically generated. However, Genomic coordinates, transcript does not change the strand. For example, NC_000011.10 and NM_003390.3 should change strand to positive, but it stays negative

@korikuzma is it okay if I create a new ticket for that? This is not something that was working or implemented like this before, so I think it's outside of the scope of this ticket. I'll create a new issue though

Can we hide the strand for now so it's not confusing and back back in #321

katiestahl added 3 commits August 12, 2024 13:12

feat!: update tx segment element with updated cool-seq-tool structure…

fa8c93c

… changes

fix: tx segment element bugs

9a6281d

update response models

fbe0a83

katiestahl added priority:high High priority build Changes that affect the build system or dependencies labels Aug 12, 2024

katiestahl requested review from jsstevenson, korikuzma and jarbesfeld August 12, 2024 19:15

katiestahl self-assigned this Aug 12, 2024

github-actions bot added the stale label Aug 14, 2024

korikuzma removed the stale label Aug 20, 2024

katiestahl added 7 commits August 20, 2024 10:45

feat: update variable names with cool-seq-tool updates

b2c6787

feat: update calls to fusor and cool-seq-tool with updated params

053843e

Merge branch 'staging' into cool-seq-tool-tx-updates

f314306

update fusor version

b58a5b7

updating fusor version

726d7e6

wip: fixing tests from cool-seq-tool and fusor updates

6c672bd

tests: updating tests with fusor changes

d56bff0

katiestahl marked this pull request as ready for review August 22, 2024 18:08

katiestahl commented Aug 22, 2024

View reviewed changes

server/tests/conftest.py Show resolved Hide resolved

katiestahl mentioned this pull request Aug 22, 2024

add back ability to infer fusion type cancervariants/fusor#183

Open

fixing test examples and adding checks for correct start/end on seque…

7191723

…nce locations

jsstevenson previously approved these changes Aug 22, 2024

View reviewed changes

jarbesfeld previously approved these changes Aug 23, 2024

View reviewed changes

update reqs

09e10bd

korikuzma dismissed stale reviews from jarbesfeld and jsstevenson via 09e10bd August 23, 2024 13:12

jarbesfeld reviewed Aug 23, 2024

View reviewed changes

katiestahl added 3 commits August 23, 2024 12:16

fix: bug where chromosome field was not editable

7f56752

Merge branch 'cool-seq-tool-tx-updates' of https://github.com/cancerv…

f26dd9f

…ariants/fusion-curation into cool-seq-tool-tx-updates

removing editable field for now since it's not working as designed, w…

a8833e4

…ill revisit at a later date

changing validation for tx segment elemetn since they only require ei…

c3d093c

…ther a start or end now, regardless of position in the structure

katiestahl commented Aug 23, 2024

View reviewed changes

katiestahl mentioned this pull request Aug 23, 2024

Chromosome field looks editable but isn't #316

Closed

korikuzma requested changes Aug 26, 2024

View reviewed changes

This was referenced Aug 26, 2024

fix: attempting to subscript an object cancervariants/fusor#184

Merged

automatically populate strand for templated sequence #320

Closed

refactor: remove ability to change strand, since it's auto populated

b611f84

katiestahl requested a review from korikuzma August 26, 2024 18:02

korikuzma mentioned this pull request Aug 27, 2024

Parallelism issue in structure tab #311

Closed

katiestahl mentioned this pull request Aug 27, 2024

autopopulate strand for tx segment when transcript is entered (not just for gene) #321

Open

katiestahl added 2 commits August 27, 2024 13:54

removing strand switch

e5545eb

fix: ability to change strand for templated sequence

09183e1

korikuzma approved these changes Aug 27, 2024

View reviewed changes

katiestahl merged commit 5ad0ec1 into staging Aug 27, 2024
5 checks passed

katiestahl deleted the cool-seq-tool-tx-updates branch August 27, 2024 18:02

korikuzma mentioned this pull request Sep 9, 2024

update fusor version #272

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: update tx segment element with updated cool-seq-tool structure changes #314

feat!: update tx segment element with updated cool-seq-tool structure changes #314

katiestahl commented Aug 12, 2024 •

edited

Loading

github-actions bot commented Aug 14, 2024

katiestahl Aug 22, 2024

katiestahl Aug 22, 2024

korikuzma Aug 22, 2024

korikuzma Aug 22, 2024

korikuzma Aug 22, 2024

katiestahl Aug 22, 2024

jsstevenson commented Aug 22, 2024

katiestahl commented Aug 22, 2024

jsstevenson commented Aug 22, 2024

jsstevenson left a comment

korikuzma commented Aug 23, 2024

jarbesfeld commented Aug 23, 2024

jarbesfeld left a comment

jarbesfeld Aug 23, 2024

jarbesfeld Aug 23, 2024

korikuzma commented Aug 23, 2024

katiestahl commented Aug 23, 2024

katiestahl commented Aug 23, 2024

katiestahl Aug 23, 2024

jarbesfeld Aug 23, 2024 •

edited

Loading

korikuzma left a comment •

edited

Loading

korikuzma commented Aug 26, 2024

korikuzma Aug 26, 2024

katiestahl Aug 26, 2024

katiestahl Aug 26, 2024

korikuzma Aug 26, 2024

katiestahl Aug 27, 2024

korikuzma Aug 27, 2024

katiestahl Aug 27, 2024

feat!: update tx segment element with updated cool-seq-tool structure changes #314

feat!: update tx segment element with updated cool-seq-tool structure changes #314

Conversation

katiestahl commented Aug 12, 2024 • edited Loading

github-actions bot commented Aug 14, 2024

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

jsstevenson commented Aug 22, 2024

katiestahl commented Aug 22, 2024

jsstevenson commented Aug 22, 2024

jsstevenson left a comment

Choose a reason for hiding this comment

korikuzma commented Aug 23, 2024

jarbesfeld commented Aug 23, 2024

jarbesfeld left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

korikuzma commented Aug 23, 2024

katiestahl commented Aug 23, 2024

katiestahl commented Aug 23, 2024

Choose a reason for hiding this comment

jarbesfeld Aug 23, 2024 • edited Loading

Choose a reason for hiding this comment

korikuzma left a comment • edited Loading

Choose a reason for hiding this comment

korikuzma commented Aug 26, 2024

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

Choose a reason for hiding this comment

katiestahl commented Aug 12, 2024 •

edited

Loading

jarbesfeld Aug 23, 2024 •

edited

Loading

korikuzma left a comment •

edited

Loading