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

Let BandageNG parse W-lines from GFA v1.1? #149

Closed
dirkjanvw opened this issue Feb 16, 2024 · 7 comments · Fixed by #153
Closed

Let BandageNG parse W-lines from GFA v1.1? #149

dirkjanvw opened this issue Feb 16, 2024 · 7 comments · Fixed by #153

Comments

@dirkjanvw
Copy link

I know that BandageNG can parse the P-line from a GFA and it's then retrieveable on the right side with "Paths..." when selecting a node. Would it be possible for BandageNG to also be able to parse W-lines and treat them the same as paths? The GFA (v1.1) files that come from the Minigraph-Cactus pipeline namely don't have P-lines but do have W-lines.

@asl
Copy link
Owner

asl commented Feb 16, 2024

This is already done in https://github.com/asl/BandageNG/tree/walks branch.

Note that walks are quite different from paths and therefore different treatment is necessary.

@dirkjanvw
Copy link
Author

dirkjanvw commented Feb 16, 2024

Cool! Thank you for pointing me to that branch, I hadn't seen it.

However, there seems to be a small bug (here: https://github.com/asl/BandageNG/blob/walks/program/main.cpp#L100) when compiling:

/Users/dirkjan/bin/BandageNG-walks/program/main.cpp:100:9: error: no matching member function for call to 'parse'
    app.parse();
    ~~~~^~~~~
/Users/dirkjan/bin/BandageNG-walks/build/_deps/cli11-src/include/CLI/impl/App_inl.hpp:593:24: note: candidate function not viable: requires single argument 'args', but no arguments were provided
CLI11_INLINE void App::parse(std::vector<std::string> &args) {
                       ^
/Users/dirkjan/bin/BandageNG-walks/build/_deps/cli11-src/include/CLI/impl/App_inl.hpp:612:24: note: candidate function not viable: requires single argument 'args', but no arguments were provided
CLI11_INLINE void App::parse(std::vector<std::string> &&args) {
                       ^
/Users/dirkjan/bin/BandageNG-walks/build/_deps/cli11-src/include/CLI/impl/App_inl.hpp:532:24: note: candidate function not viable: requires 2 arguments, but 0 were provided
CLI11_INLINE void App::parse(int argc, const char *const *argv) { parse_char_t(argc, argv); }
                       ^
/Users/dirkjan/bin/BandageNG-walks/build/_deps/cli11-src/include/CLI/impl/App_inl.hpp:533:24: note: candidate function not viable: requires 2 arguments, but 0 were provided
CLI11_INLINE void App::parse(int argc, const wchar_t *const *argv) { parse_char_t(argc, argv); }
                       ^
/Users/dirkjan/bin/BandageNG-walks/build/_deps/cli11-src/include/CLI/impl/App_inl.hpp:558:24: note: candidate function not viable: requires at least argument 'commandline', but no arguments were provided
CLI11_INLINE void App::parse(std::string commandline, bool program_name_included) {
                       ^
/Users/dirkjan/bin/BandageNG-walks/build/_deps/cli11-src/include/CLI/impl/App_inl.hpp:589:24: note: candidate function not viable: requires at least argument 'commandline', but no arguments were provided
CLI11_INLINE void App::parse(std::wstring commandline, bool program_name_included) {
                       ^
1 error generated.
make[2]: *** [CMakeFiles/BandageNG.dir/program/main.cpp.o] Error 1
make[1]: *** [CMakeFiles/BandageNG.dir/all] Error 2
make: *** [all] Error 2

edit: link should be to branch walks instead of dev

@asl
Copy link
Owner

asl commented Feb 16, 2024

Cool! Thank you for pointing me to that branch, I hadn't seen it.

However, there seems to be a small bug (here: https://github.com/asl/BandageNG/blob/dev/program/main.cpp#L100) when compiling:

Unfortunately, the dependency (CLI11 lib) changed after branch was created in non-backward compatible way. So you'd either need to roll-back to previous version or fix the problem. Using https://github.com/CLIUtils/CLI11/releases/tag/v2.3.2 is probably the easiest way, for this you'd need to change CMakeLists.txt:

FetchContent_Declare(cli11 GIT_REPOSITORY https://github.com/CLIUtils/CLI11 GIT_TAG v2.3.2)
FetchContent_MakeAvailable(cli11)

(note v2.3.2 as tag name instead of main)

@dirkjanvw
Copy link
Author

Hmm that solution gives the same compilation error for me. This one solves it though:

diff --git a/program/main.cpp b/program/main.cpp
index b143b8e..669a22c 100644
--- a/program/main.cpp
+++ b/program/main.cpp
@@ -59,7 +59,7 @@ using SubCmd = std::variant<std::monostate,
                             QueryPathsCmd,
                             LayoutCmd>;
 
-static SubCmd parseCmdLine(CLI::App &app) {
+static SubCmd parseCmdLine(CLI::App &app, int argc, char *argv[]) {
     SubCmd subcmd;
 
     app.description(getBandageTitleAsciiArt() + '\n' +
@@ -97,7 +97,7 @@ static SubCmd parseCmdLine(CLI::App &app) {
 
     app.footer("Online Bandage help: https://github.com/asl/BandageNG/wiki");
 
-    app.parse();
+    app.parse(argc, argv);
 
     if (app.got_subcommand(load)) {
         g_memory->commandLineCommand = BANDAGE_LOAD; // FIXME: not needed
@@ -149,7 +149,7 @@ int main(int argc, char *argv[]) {
     g_settings.reset(new Settings());
 
     try {
-        cmd = parseCmdLine(cli);
+        cmd = parseCmdLine(cli, argc, argv);
     } catch (const CLI::ParseError &e) {
         return cli.exit(e);
     }

With this I can compile, load and draw my Minigraph-Cactus graph, but I cannot find any of the W-lines in the graph. I tried drawing one walk only, selecting nodes from an entire graph and clicking "Paths...", and searching one walk with "Find path". Is there a way for me to try debugging this (a --verbose or --debug options)?

@asl
Copy link
Owner

asl commented Feb 17, 2024

Hmm that solution gives the same compilation error for me. This one solves it though:

You'd also need to wipe out what was already fetched and compiled. This is how cmake works, sadly.

and clicking "Paths..."

You should use "Walks", not "Paths". Likely you're using wrong branch or so.

@dirkjanvw
Copy link
Author

Ah yeah sorry, you are right. After checking out the correct branch I see it. I do get a segmentation error on my macOS when loading the GFA file (and I am struggling to compile on Linux), so I'll look into this later :) Do you have any plans to include this in a future release?

@asl
Copy link
Owner

asl commented Feb 17, 2024

We should never segfault even on invalid input. Can you open separate issue with example gfa?

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 a pull request may close this issue.

2 participants