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

Parameter flagged as missing even though it appears #5

Closed
rob-p opened this issue Nov 30, 2017 · 11 comments
Closed

Parameter flagged as missing even though it appears #5

rob-p opened this issue Nov 30, 2017 · 11 comments
Assignees

Comments

@rob-p
Copy link

rob-p commented Nov 30, 2017

Hi. I get the following behavior using the latest version of clipp. The issue seems to be that the required options are only properly parsed if they appear in a certain order. Consider the following small test program:

#include "clipp.h"
#include <iostream>

int main(int argc, char* argv[]) {
  using namespace clipp;
  enum class mode {help, index};
  mode selected = mode::help;
  std::string odir;
  std::string gfaFile;
  std::string rfile;
  int k{31};
  bool isSparse{false};
  int extensionSize{4};

  auto indexMode = (
                    command("index").set(selected, mode::index),
                    (required("-o", "--output") & value("output dir", odir)) % "directory where index is written",
                    (required("-g", "--gfa") & value("gfa file",gfaFile)) % "path to the GFA file",
                    (required("-r", "--ref") & value("ref file",rfile)) % "path to the reference fasta file",
                    (option("-k", "--klen") & value("kmer length",k))  % "length of the k-mer with which the dBG was built (default = 31)",
                    (option("-s", "--sparse").set(isSparse, true)) % "use the sparse pufferfish index (less space, but slower lookup)",
                    (option("-e", "--extension") & value("extension size",extensionSize)) % "length of the extension to store in the sparse index (default = 4)"
                    );

auto cli = (
              (indexMode | command("help").set(selected,mode::help) ),
              option("-v", "--version").call([]{std::cout << "version 0.1.0\n\n";}).doc("show version"));

  decltype(parse(argc, argv, cli)) res;
  try {
    res = parse(argc, argv, cli);
  } catch (std::exception& e) {
    std::cout << "\n\nParsing command line failed with exception: " << e.what() << "\n";
    std::cout << "\n\n";
    std::cout << make_man_page(cli, "x");
    return 1;
  }


  if(res) {
    switch(selected) {
    case mode::index: std::cout << "successful; call index\n";  break;
    case mode::help: std::cout << make_man_page(cli, "x"); break;
    }
  } else {
    auto b = res.begin();
    auto e = res.end();
    std::cerr << "any blocked " << res.any_blocked() << "\n";
    std::cerr << "any conflict " << res.any_conflict() << "\n";
    std::cerr << "any bad repeat " << res.any_bad_repeat() << "\n";
    std::cerr << "any error " << res.any_error() << "\n";
    for( auto& m : res.missing() ) {
      std::cerr << "missing " << m.param()->label() << "\n";
    }
    if (std::distance(b,e) > 0) {
      if (b->arg() == "index") {
        std::cout << make_man_page(indexMode, "x");
      } else {
        std::cout << "There is no command \"" << b->arg() << "\"\n";
        std::cout << usage_lines(cli, "x") << '\n';
        return 1;
      }
    } else {
      std::cout << usage_lines(cli, "x") << '\n';
      return 1;
    }
  }
return 0;
}

When I pass all of the required options as follows, the parser fails. All of the options do look to have the appropriate associated value, but the "gfa_file" option is marked as missing even though it is clearly present.

rob@server:~/main/build$ ./main index -g valid_file.gfa -o out_dir -r rfile 
any blocked 0
any conflict 0
any bad repeat 0
any error 1
missing
missing gfa_file
SYNOPSIS
        main index -o <output_dir> -g <gfa_file> -r <ref_file> [-k <kmer_length>] [-s] [-e <extension_size>]

OPTIONS
        -o, --output <output_dir>
                    directory where index is written

        -g, --gfa <gfa_file>
                    path to the GFA file

        -r, --ref <ref_file>
                    path to the reference fasta file

        -k, --klen <kmer_length>
                    length of the k-mer with which the dBG was built (default = 31)

        -s, --sparse
                    use the sparse pufferfish index (less space, but slower lookup)

        -e, --extension <extension_size>
                    length of the extension to store in the sparse index (default = 4)
rob@server:~/main/build$

Strangely, if I simply change the order of the named options, everything works as expected:

rob@server:~/main/build$ ./main index -o out_dir -g valid_file.gfa -r rfile 
successful; call index
rob@server:~/main/build$

Presumably, both of these invocations should be equivalent. Any idea why the former is failing?

Thanks!
Rob

[Edit: In case it's helpful, I found the function you have to print the parsing results in the debug namespace. Here is what it gives with the example that fails]

#1 index -> index
#2 -g -> -g
#3 /mnt/scratch7/pufferfish_data/k31_n_gencode.v25.pc_transcripts_fixed.pufferized.gfa -> gfa_file
#4 -o -> -o
#5 test_new_parser -> output_dir
#6 -r -> -r
#7 /mnt/scratch7/gencode.v25.pc_transcripts_fixed.fa -> ref_file
-g       [missing after 4]
gfa_file         [missing after 4]
@muellan
Copy link
Owner

muellan commented Dec 1, 2017

Hi.
That's obviously a bug in the missing param handling.
I will look into it (...and already have a feeling where this is going wrong...).

BTW: I saw that you enclosed the parsing in a try/catch block. I'm curious: Is that because you are just beeing careful? The only exceptions that could be thrown are those escaping from std library calls (like std::bad_alloc). I don't throw explicitly in clipp, because I don't want to require exception handling (some people need to compile without it).

@rob-p
Copy link
Author

rob-p commented Dec 1, 2017

The try / catch is because, in the longer-form version of this code, I actually have some callbacks that can throw exceptions (e.g. to create a directory if it doesn't exist, which might throw if the user doesn't have permission etc.). I'm not operating in any scenario where I can't afford exceptions. The main software we work on is high-performance genomics software run on mostly linux and osx x86-64 servers.

@rob-p
Copy link
Author

rob-p commented Dec 3, 2017

Any thoughts on this? I took a brief look over the code, but don't think I grok the basic design well enough to diagnose the error.

@jube
Copy link

jube commented Dec 3, 2017

I have the same kind of problem. Here is an example:

#include <iostream>

#include <clipp.h>

int main(int argc, char *argv[]) {

  std::string seed;
  std::string prog;

  auto cli = (
    clipp::option("-s", "--seed").doc("set the seed value") & clipp::value("seed", seed),
    clipp::value("player", prog)
  );

  auto res = parse(argc, argv, cli);

  if(!res) {
    clipp::debug::print(std::cerr, res);
    std::cerr << clipp::make_man_page(cli, argv[0]) << '\n';
    return -1;
  }

  std::cout << "Seed: " << seed << '\n';
  std::cout << "Player: " << prog << '\n';
  return 0;
}

And the result:

$ ./bug ./foo
Seed: 
Player: ./foo
$ ./bug -s 123 ./foo
#1 -s -> -s 
#3 123./foo -> seed 
player   [missing after 0]
SYNOPSIS
        ./bug [-s <seed>] <player>

OPTIONS
        -s, --seed  set the seed value

$ ./bug -s 123 foo
Seed: 123
Player: foo

Adding a point in the last argument makes the parser crazy.

@muellan
Copy link
Owner

muellan commented Dec 3, 2017

I was quite busy the past few days and traveled a lot. I should find the time to get it fixed in the next days (both the requirement bug and the one with the point).

@muellan
Copy link
Owner

muellan commented Dec 4, 2017

OK, fixed the bug with parameters starting with ..
I also added new tests for the problem with the missing events for required parameters and found the cause of the bug. Should be fixed soon.

@rob-p
Copy link
Author

rob-p commented Dec 5, 2017

Cool! It looks like the fix has not been pushed yet, right? (or are you just waiting until both are fixed?)

@muellan
Copy link
Owner

muellan commented Dec 5, 2017

I pushed the fix.

@muellan muellan added the bug label Dec 5, 2017
@muellan muellan self-assigned this Dec 5, 2017
@muellan muellan added fixed and removed fixed labels Dec 6, 2017
@muellan
Copy link
Owner

muellan commented Dec 6, 2017

Ah crap. I missed some cases where it is still not working correctly...

@muellan
Copy link
Owner

muellan commented Dec 7, 2017

So - finally did it. Fixing the first bug uncovered another one. Both should be fixed by now.

@muellan muellan added the fixed label Dec 7, 2017
@rob-p
Copy link
Author

rob-p commented Dec 7, 2017

Awesome! The newest release definitely solves the specific issue I posted, so I'll update my version of clipp across my projects currently using it. Thanks!

@rob-p rob-p closed this as completed Dec 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants