Prints to loggers #684
Prints to loggers #684
Conversation
|
Overall looks good. But the tests are failing because of PEP8 formatting errors. Perhaps it would be too much work to enforce PEP8 style on all the files you're touching. But I beleive right now only the file length limit is enforced. So you should be good only by breaking the lines longer than 79 characters. |
|
One more comment. No need to add logger to test files, please ignore them. |
|
So, do you want me to undo what I've done in these files or just ignore the next test files? |
|
Just ignore the next test files. |
|
Ok! One more question: Ian used stuff like self.verbose and debug in his files, since we can control these levels with logging, should I remove them? |
|
Yeah, it make sense to remove them. Just in case, @goodfeli do you agree? |
|
Just a message to say that I went through all the files. |
| "the try/catch so you can see whatever error it prints on " | ||
| "its own.") | ||
| logger.error("Couldn't open '%s' and exception has no string. " + | ||
| "Opening it again outside the try/catch " + |
There was a problem hiding this comment.
Maybe logger exception would be better here, since the code is not halting yet.
| For example, | ||
| pkl_inspector.py foo.pkl .my_field [my_key] 3 | ||
| will load an object obj from foo.pkl and analyze obj.my_field["my_key"][3] | ||
| """ | ||
| """) |
There was a problem hiding this comment.
Shouldn't all these be Indented?
|
About the indentation of multiline strings: I wanted to keep the formatting and there weren't indented originally. If we indent them, the text will be indented in terminal too. |
| "may randomly fail later") | ||
| logger.error("the fact that somebody is using this doesn't bode well " | ||
| "since it's unlikely that the " | ||
| "covariance matrix is sparse") |
There was a problem hiding this comment.
Here again, the code is not halting, so maybe exception or warning
|
About indention, I personally usually do something like: |
|
I corrected what you mentioned (the training error confused me!). |
|
I mistakenly deleted my last commit. |
Revert "Merge pull request #684 from Menerve/prints_to_loggers"
I replaced prints statements by logging in some parts of Pylearn2 (work in progress) to resolve the issue #41
I tried to keep the form of what the author wanted to print but I also keep the technic used to print the string (some use str('string') and concatenation, others the modulo operator). The modulo operator can be a problem when I can't know the type of the value at first glance, so it might be interesting to avoid it.
There isn't a lot of logger.debug and I put it where it seems obvious, but maybe, at some place, logger.debug is better than logger.info. Let me know.