Conversation
So, this is actually pretty close to being ready. HOWEVER! It turns out that there are quite a lot of paths in these files that can span multiple connected components of the graph, which is crazy to me. My code expects a path to be constrained to only a single connected component, because ... it's a path Anyway, I can definitely adjust things to allow a path to map to multiple ccs -- this would just mean that we would not consider a path as "available" unless all of the ccs that it spans are currently drawn. Definitely feasible. This would take some time, though. And in the interest of getting a release out soon AND in actually testing this functionality, I think it is best to consign this feature to another-git-branch jail until I get it sorted out.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #408 +/- ##
==========================================
+ Coverage 65.15% 65.73% +0.58%
==========================================
Files 33 33
Lines 3980 4039 +59
Branches 975 989 +14
==========================================
+ Hits 2593 2655 +62
- Misses 1311 1312 +1
+ Partials 76 72 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
the format is very similar to flye info so probs will abstract some of that code to a shared util func
buhlhfbluhblu
need to elsewhere adjust things to support multi-cc paths + clean up code + test
fedarko
changed the title
will add diff files for testing later using tsv probs
urgh my head hurts
this way, changes to the internal implementation (to allow for multi-cc paths) have a limited impact
and just like that, verkko paths are pretty much sorted! Something to think about: should we address #357? There are some paths left out when we draw all NR ccs (#67). If we duplicate each path like we do for GFA paths, then this should fix the problem for single-component paths. But what about paths that traverse multiple components? Ugh, things get complicated. Especially if there is jank like a path that traverses BOTH a component and its twin, in which case we can never represent this path when drawing all NR ccs!!! oh no!!!! The trouble with duplicating paths is that - at least from my perspective - it gets you thinking about "okay, 20 / 98 paths are available, how many of those missing 78 paths are 'real ones'?" Like, the user doesn't care about missing paths if they are just perfect RCs of the currently available paths. And how do you communicate that through just a number...? After writing all of this out, I think a better solution to #357 is just NOT duplicating paths at all - even for GFA files. This way, if the user draws some components and sees that some paths are not available, then it is clear that all the missing paths have some inherent meaning (and are not perfect RCs of the available paths). Some paths may be missing even when drawing NR components, in which case the user can fix the problem by drawing all components. I think this is very reasonable? We could even maybe go crazy and try to set which component is the drawn one for NR based on how many paths cross it, but... that seems not worth it. Just keep things simple................
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #336 and closes #357. This has the nice side effect of allowing multi-component paths (across all path filetypes, not just Verkko TSVs).