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

General update to logging. #382

Merged
merged 1 commit into from
Mar 23, 2014
Merged

General update to logging. #382

merged 1 commit into from
Mar 23, 2014

Conversation

ketch
Copy link
Member

@ketch ketch commented Mar 19, 2014

Make all loggers inherit from pyclaw logger.
Add convenience function for setting console logging level in controller (h/t @ahmadia).

I see two main use cases where one wants to adjust the logging:

  1. Turn off logging to stdout (IPython notebooks)
  2. Turn off all logging (or set all to CRITICAL) (Shaheen)

For 1, we want to selectively modify the level of the console handler for the controller logger. This can now be done via

claw.verbosity = 0

Meanwhile, the controller will still write to the log file.

For case 2, one can do

import logging
logger = logging.getLogger('pyclaw')
logger.setLevel(logging.CRITICAL)

which propagates to the other loggers.

For completeness, the logger name changes are:

io -> pyclaw.io
solution -> pyclaw.solution
evolve -> pyclaw.solver

and there is a new logger pyclaw.controller.

@ahmadia
Copy link
Member

ahmadia commented Mar 19, 2014

This is currently failing in Travis:

..........................S..E
======================================================================
ERROR: Failure: ValueError (Attempted relative import beyond toplevel package)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages/nose/loader.py", line 413, in loadTestsFromName
    addr.filename, addr.module)
  File "/home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages/nose/importer.py", line 47, in importFromPath
    return self.importFromDir(dir_path, fqname)
  File "/home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages/nose/importer.py", line 94, in importFromDir
    mod = load_module(part_fqname, fh, filename, desc)
  File "/home/travis/build/clawpack/pyclaw/src/pyclaw/__init__.py", line 23, in <module>
    from .controller import Controller
  File "/home/travis/build/clawpack/pyclaw/src/pyclaw/controller.py", line 20, in <module>
    from ..pyclaw import LOGGING_LEVELS
ValueError: Attempted relative import beyond toplevel package
----------------------------------------------------------------------
Ran 30 tests in 167.205s

Make all loggers inherit from pyclaw logger.
Add convenience function for setting console logging level
in controller (h/t @ahmadia).
@ketch
Copy link
Member Author

ketch commented Mar 20, 2014

@ahmadia thanks for the heads up. I've fixed it now.

@mandli
Copy link
Member

mandli commented Mar 20, 2014

Is there a way to merge the functionality of replace_stream_handlers in runclaw.py, it would be nice to have a consistent method of interacting with the loggers.

@ketch
Copy link
Member Author

ketch commented Mar 21, 2014

Can you explain why you use replace_stream_handlers? It seems like the same thing can be accomplished more easily and cleanly with this PR, but I may have missed something.

@mandli
Copy link
Member

mandli commented Mar 21, 2014

Actually, I was just hoping that you could outline how we would replace that function with this PR. If there's anything missing than perhaps we could include it.

@ketch
Copy link
Member Author

ketch commented Mar 22, 2014

I don't think this PR should change replace_stream_handlers. It does change a couple of logger names, so we'll need to make the corresponding changes in the places where replace_stream_handlers is called.

Honestly, after a couple of hours of reading, Python's logging module doesn't make much sense. It's extremely unpythonic -- there are 100 ways to do anything, and none is clearly the best.

@ahmadia
Copy link
Member

ahmadia commented Mar 22, 2014

@ketch - I think it's designed to support a very different use case than the one most scientists are interested in.

@ahmadia
Copy link
Member

ahmadia commented Mar 22, 2014

This looks like it's passing tests, are you ready to have this merged in?

@mandli
Copy link
Member

mandli commented Mar 22, 2014

I am fine merging it in now, I did not want to duplicate functionality but it sounds like it won't be that much of an issue.

ketch added a commit to ketch/clawapps that referenced this pull request Mar 23, 2014
mandli added a commit that referenced this pull request Mar 23, 2014
@mandli mandli merged commit aa07df9 into clawpack:master Mar 23, 2014
@ketch ketch deleted the logging_update branch June 14, 2014 14:32
mandli pushed a commit to mandli/multilayer-examples that referenced this pull request May 1, 2015
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.

None yet

3 participants