pepkit / looper

A job submitter for Portable Encapsulated Projects
http://looper.databio.org
BSD 2-Clause "Simplified" License
20 stars 7 forks source link

looper debug option is missing #328

Closed nsheff closed 1 year ago

nsheff commented 3 years ago

I can't do looper --dbg anymore.

stolarczyk commented 3 years ago

I've commented out the Logging control block for some reason, not sure why. And then pushed these changes here by mistake

https://github.com/pepkit/looper/blob/8dd6b432385ce8417aaeb6d253d2d08fa8199e82/looper/__init__.py#L105-L116

nsheff commented 3 years ago

The only way I can get it to work is looper --verbosity=5 run pep_bio.yaml

nsheff commented 3 years ago

I've commented out the Logging control block for some reason, not sure why. And then pushed these changes here by mistake

Oh okay, got it!

nsheff commented 3 years ago

if you uncomment that though you get:

nsheff@zither:~/code/incubator/refgenie_looper_demo$ looper --dbg run pep_bio.yaml 
Traceback (most recent call last):
  File "/home/nsheff/.local/bin/looper", line 8, in <module>
    sys.exit(main())
  File "/home/nsheff/.local/lib/python3.8/site-packages/looper/looper.py", line 1003, in main
    parser = logmuse.add_logging_options(parser)
  File "/home/nsheff/.local/lib/python3.8/site-packages/logmuse/est.py", line 75, in add_logging_options
    parser.add_argument("--{}".format(optname), **optdata)
  File "/usr/lib/python3.8/argparse.py", line 1398, in add_argument
    return self._add_action(action)
  File "/usr/lib/python3.8/argparse.py", line 1761, in _add_action
    self._optionals._add_action(action)
  File "/usr/lib/python3.8/argparse.py", line 1602, in _add_action
    action = super(_ArgumentGroup, self)._add_action(action)
  File "/usr/lib/python3.8/argparse.py", line 1412, in _add_action
    self._check_conflict(action)
  File "/usr/lib/python3.8/argparse.py", line 1551, in _check_conflict
    conflict_handler(action, confl_optionals)
  File "/usr/lib/python3.8/argparse.py", line 1560, in _handle_conflict_error
    raise ArgumentError(action, message % conflict_string)
argparse.ArgumentError: argument --verbosity: conflicting option string: --verbosity
nsheff commented 3 years ago

because it's added here: https://github.com/pepkit/looper/blob/7c7816de4035dacf91e5be479bfb8babfae3ca78/looper/looper.py#L1003

stolarczyk commented 3 years ago

ok so this is why I commented it out..

Now I see that this commit https://github.com/pepkit/looper/commit/ae0766f35a98578c577c52d1611192213ef2b3fa aimed to simplify the logging setup and use logmuse for that. Interestingly, it follows refgenie's setup, where --verbosity X works just fine.

This might be related to the auxiliary parser concept in looper.

nsheff commented 3 years ago

Yeah, looper predated logmuse so it was never really fully integrated. So, standardizing this is a good idea in general, but it looks like you started that about a month ago but got sidetracked.

stolarczyk commented 3 years ago

alright, I brought it back to how it was set up before, so it isn't a blocking issue.

We will need to try to use logmuse for that in the future.

ayobi commented 1 year ago

Still wrapping my head around everything but browsing through I see logmuse in use in dev branch in __init__.py and looper.py https://github.com/pepkit/looper/blob/b0e5918c49c4564a2eb637ae4c8d2f9fbfd35c72/looper/__init__.py#L10-L12 https://github.com/pepkit/looper/blob/b0e5918c49c4564a2eb637ae4c8d2f9fbfd35c72/looper/looper.py#L1037-L1056 and running a --dbg command it indicated quite a few project are configured using logmuse: aaronobrien@databio:~/hello_looper-master$ looper --dbg run project/project_config.yaml DEBU 11:30:45 | peppy:est:310 > Configured logger 'peppy' using logmuse v0.2.7 DEBU 11:30:45 | divvy:est:310 > Configured logger 'divvy' using logmuse v0.2.7 DEBU 11:30:45 | eido:est:310 > Configured logger 'eido' using logmuse v0.2.7 DEBU 11:30:45 | pipestat:est:310 > Configured logger 'pipestat' using logmuse v0.2.7 DEBU 11:30:45 | looper:est:310 > Configured logger 'looper' using logmuse v0.2.7 INFO 11:30:45 | looper:looper:1056 > Looper version: 1.4.0

I see in other scripts logger is used. Are we looking to replace logger with logmuse to other scripts or is that unnecessary? Or are we trying to replace the current parser: https://github.com/pepkit/looper/blob/df535be2a0bd7257ee37148cb601d688ec557909/looper/__init__.py#L73-L93

Since I see in the logmuse tutorial:

When you build your argparser, add the logmuse CLI options with this:

parser = logmuse.add_logging_options(parser)

khoroshevskyi commented 1 year ago

Answering your question:

I see in other scripts logger is used. Are we looking to replace logger with logmuse to other scripts or is that unnecessary? Or are we trying to replace the current parser?

We want to add logmuse only to looper. Some of the other scripts are already using logmuse, and other don't need it.

But I think, we want to keep option, that configures level of verbosity of loggers in other scirpts. Correct me if I am wrong.

ayobi commented 1 year ago

After digging deeper, I believe I was able to get it to work. I'll push my changes.

nsheff commented 1 year ago

It appears that we integrated logmuse effectively; this did remove the --dbg option but I think that's for the better for now. If we want that it should be integrated in logmuse.