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

Improvements to Supplemental Read navigation and display #1412

Merged
merged 1 commit into from
Nov 16, 2023

Conversation

lbergelson
Copy link
Contributor

Major rework of the previous alignment dialog popup to make it interactive and clearer. Use it to navigate between reads in a supplemetnary alignment group.
Click to move to the selected read.
Shift + Click to add it as a split screen.

When jumping to a selected read/mate the selected reads will be now be sorted to the top.
@jrobinso There's currently a nondeterministic bug here, sometimes the sorting crashes with an NPE because the interval isn't loaded. I'm misunderstanding something about the loading code. It would be easy to fix it so it doesn't blow up by checking the interval for null, but then we'd just have the problem that it sometimes isn't putting the selected reads at the top mysteriously. I left it to blow up so that it's obvious when it happens so we can notice / fix it. I'm a bit puzzled by it

java.lang.NullPointerException: Cannot invoke "org.broad.igv.sam.AlignmentInterval.sortRows(org.broad.igv.sam.SortOption, double, String, boolean, java.util.Set)" because "interval" is null
        at java.base/java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:315)
        at java.base/java.util.concurrent.CompletableFuture.completeThrowable(CompletableFuture.java:320)
        at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1807)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: java.lang.NullPointerException: Cannot invoke "org.broad.igv.sam.AlignmentInterval.sortRows(org.broad.igv.sam.SortOption, double, String, boolean, java.util.Set)" because "interval" is null
        at org.broad.igv.sam.AlignmentTrack.lambda$sortRows$2(AlignmentTrack.java:767)
        at org.broad.igv.sam.AlignmentTrack.lambda$receiveEvent$1(AlignmentTrack.java:402)
        at java.base/java.util.HashMap.computeIfPresent(HashMap.java:1261)
        at org.broad.igv.sam.AlignmentTrack.receiveEvent(AlignmentTrack.java:401)
        at org.broad.igv.event.IGVEventBus.post(IGVEventBus.java:83)
        at org.broad.igv.sam.AlignmentDataManager.load(AlignmentDataManager.java:361)
        at org.broad.igv.sam.CoverageTrack.load(CoverageTrack.java:185)
        at org.broad.igv.ui.IGV.lambda$repaint$16(IGV.java:2338)
        at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1804)
        ... 3 more

Renamed several classes and moved them to the ui/supdiagram package Refactoring some methods around Frame creation, this could still be improved.

@jrobinso
Copy link
Contributor

@lbergelson when does this lambda function (sort) ever get removed from the actionToPerformOnFrameLoad cache? The purpose seems to be to defer sorting for this frame, but as far as I can see this never gets removed. Won't it get invoked repeatedly, rather than once (which seems to be the intent).

This doesn't seem to be added in this PR, I just noticed it looking into the bug you posted above.

actionToPerformOnFrameLoad.put(frame, sort);

@lbergelson
Copy link
Contributor Author

@jrobinso It should be removed when it's executed. If you look at where it's called, it's in a "computeIfPresent" block and after executing it returns null which removes it. I could replace it which a remove() and != null check if you think that's more readable.

…d navigation.

Major rework of the previous alignment dialog popup to make it interactive and clearer.
Use it to navigate between reads in a supplemenary alignment group.
   Click to move to the selected read.
   Shift + Click to add it as a split screen.

When jumping to a selected read/mate the selected reads will be now be sorted to the top.

Renamed several classes and moved them to the ui/supdiagram package
Refactoring some methods around Frame creation, this could still be improved.
@jrobinso jrobinso merged commit fc5d461 into master Nov 16, 2023
2 checks passed
@jrobinso jrobinso deleted the lb_label_popup branch November 16, 2023 02:20
jrobinso pushed a commit that referenced this pull request Dec 4, 2023
…d navigation. (#1412)

Major rework of the previous alignment dialog popup to make it interactive and clearer.
Use it to navigate between reads in a supplemenary alignment group.
   Click to move to the selected read.
   Shift + Click to add it as a split screen.

When jumping to a selected read/mate the selected reads will be now be sorted to the top.

Renamed several classes and moved them to the ui/supdiagram package
Refactoring some methods around Frame creation, this could still be improved.
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