ENH: Replace print calls with Logger.log
#86
Conversation
This file contains 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
BvB93
commented
May 25, 2022
| @@ -50,21 +53,20 @@ def report(self): | |||
| self.out << "[ERROR!]\n\n" | |||
|
|
|||
| for job, e in self.errors: | |||
| msg = 'ERROR ' | |||
There was a problem hiding this comment.
No need to manually prefix ERROR if it's already included via the logging level.
Comment on lines
+101
to
+104
| logger.critical( | ||
| "Internal error encountered. Contact the developers.", | ||
| exc_info=exc_val, | ||
| ) |
There was a problem hiding this comment.
An example of a logging message where a traceback will be included.
There was a problem hiding this comment.
An example of such a tracebacked logging output from https://github.com/SCM-NV/nano-qmflows/runs/6536779272?check_suite_focus=true:
Click to expand logging output
INFO nanoqm:input_validation.py:191 path to basis added to cp2k settings
INFO nanoqm:initialization.py:59 Using nano-qmflows version: 0.12.1
INFO nanoqm:initialization.py:60 nano-qmflows path is: /opt/hostedtoolcache/Python/3.10.4/x64/lib/python3.10/site-packages/nanoqm
INFO nanoqm:initialization.py:61 Working directory is: /tmp/nanoqm
INFO nanoqm:initialization.py:62 Data will be stored in HDF5 file: /tmp/pytest-of-runner/pytest-0/test_workflow_IPR0/Cd33Se33.hdf5
INFO nanoqm:initialization.py:177 An initial Calculation will be computed as guess for the wave function
INFO nanoqm:tools.py:22 Starting single point calculation.
INFO nanoqm:initialization.py:59 Using nano-qmflows version: 0.12.1
INFO nanoqm:initialization.py:60 nano-qmflows path is: /opt/hostedtoolcache/Python/3.10.4/x64/lib/python3.10/site-packages/nanoqm
INFO nanoqm:initialization.py:61 Working directory is: /tmp/nanoqm
INFO nanoqm:initialization.py:62 Data will be stored in HDF5 file: /tmp/pytest-of-runner/pytest-0/test_workflow_IPR0/Cd33Se33.hdf5
INFO nanoqm:initialization.py:177 An initial Calculation will be computed as guess for the wave function
INFO nanoqm:workflow_single_points.py:41 starting!
INFO nanoqm:components.py:114 point_0 has been scheduled
INFO nanoqm:components.py:224 Total energy of point 0 is: -1832.3239105553077
ERROR noodles:scheduler.py:105 Uncaught error running job 140216626697056
Traceback (most recent call last):
File "/opt/hostedtoolcache/Python/3.10.4/x64/lib/python3.10/site-packages/noodles/run/worker.py", line 34, in run_job
result = node.apply()
File "/opt/hostedtoolcache/Python/3.10.4/x64/lib/python3.10/site-packages/noodles/workflow/model.py", line 57, in apply
return self.foo(*self.bound_args.args, **self.bound_args.kwargs)
File "/opt/hostedtoolcache/Python/3.10.4/x64/lib/python3.10/site-packages/nanoqm/schedule/components.py", line 159, in store_molecular_orbitals
save_orbitals_in_hdf5(mos, config, dict_input["job_name"])
File "/opt/hostedtoolcache/Python/3.10.4/x64/lib/python3.10/site-packages/nanoqm/schedule/components.py", line 173, in save_orbitals_in_hdf5
dump_orbitals_to_hdf5(mos, config, job_name)
File "/opt/hostedtoolcache/Python/3.10.4/x64/lib/python3.10/site-packages/nanoqm/schedule/components.py", line 202, in dump_orbitals_to_hdf5
store_arrays_in_hdf5(config.path_hdf5, path_property, array)
File "/opt/hostedtoolcache/Python/3.10.4/x64/lib/python3.10/site-packages/nanoqm/common.py", line 288, in store_arrays_in_hdf5
dset = f5.require_dataset(paths, shape=np.shape(
File "/opt/hostedtoolcache/Python/3.10.4/x64/lib/python3.10/site-packages/h5py/_hl/group.py", line 239, in require_dataset
raise TypeError("Shapes do not match (existing %s vs new %s)" % (dset.shape, shape))
TypeError: Shapes do not match (existing (80,) vs new (100,))
Comment on lines
+97
to
+98
| for handler in logger.handlers: | ||
| handler.flush() |
There was a problem hiding this comment.
I'm not even 100% sure if flushing is required for the stderr stream, but for now I'm just mimicking the previous print semantics as close as possible.
Comment on lines
+21
to
+26
| stream_handler = logging.StreamHandler() | ||
| stream_handler.setLevel(logging.INFO) | ||
| stream_handler.setFormatter(logging.Formatter( | ||
| fmt="[%(asctime)s] %(levelname)s: %(message)s", | ||
| datefmt="%H:%M:%S", | ||
| )) |
There was a problem hiding this comment.
By default logging.StreamHandler will output to stderr. Other streams (stdout) are also a possiblity, or something like picking a stream based on the level, but for the sake of simplicity I personally have a slight preference for stderr.
Any thoughts?
Comment on lines
-126
to
-128
| for error in errors: | ||
| print("Exception of type", type(error), ":") | ||
| print(error) |
There was a problem hiding this comment.
All exception info is already included in the preceding logger.error call, so no need to repeat it here.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 #85
This PR replaces the various
printcalls throughout the library with equivalent(-ish)logging.logcalls, the latter having the advantage of greater user-control of the printed output and the which stream(s) are used therefor.A best effort has been made to pick appropriate logging levels based on contact clues, .e.g a message previously being prefixed with "WARNING: " or, in the case of
CRITICAL, a message mentioning an internal error and the user should contact the developers. Furthermore, logging messages based on a type of caught exception now also include the exceptions traceback, as these are in my experience extremely usefull when debugging exceptions.