Consistent use of logger?

[DrDomenicoMarson]

I was trying to figure out how to make better use of logger/raise in the AMBER parser, and I checked other files in the project, so I took the advance and made a list of something I observed.

I have some comments for these files:

• parsing/amber --> some Exceptions handled with logger.exception without raising a proper exception (this I have to change in issue AMBER: inconsistent use of raise Exception / logger.exception #227

• parsing/gmx --> no logger, but also no raise (it's impossible to get an error while reading a gmx file?)

• parsing/gomc --> no logger, but also no raise (no possible errors in the file?)

• parsing/namd --> coherent, lot of logger.error/warning followed directly with a raise (maybe missing one at lines 81 and 239)

• parsing/util --> no logger, but we have a raise ValueError without logging the error at line 79

• postprocessors/units --> no logger, but we have different raise SOMETHING, without logging

• preprocessing/subsampling --> no logger, but we have different raise SOMETHING, without logging

• visualisation/* --> no logger, but we have different raise SOMETHING, without logging

• init/concat --> no logger, just raised two ValueError without logging

I don't know if everything here is what is supposed to be, I'm new to the use of the logging module, I just wanted to report where I saw many raise without the logging of some information.

Also, I noticed in some modules more than others we are extensively using logger.info, while in many more modules logger.info is never used, what's the rationale behind the decision to log something to info?
info is printed just if we increase the level of logging information right? Are we doing so in some part of the code?

[xiki-tempula]

The package was started a long time ago and various functions are added by various people across a very long period so the logging/exception/warning is not set up in a uniformed standard.

My feeling is that if there is something that you wish the user to know it will be through logging.info
If there is a warning, then it should be

logging.warning(msg)
warning.warn(msg)

If there is an exception, it should be

logging.exception(msg)
raise Exception(msg)

These are the ideal standards but I'm well aware that almost no functions obey this.

[DrDomenicoMarson]

Great, I'll try to follow the more standard practice with my commits then!

[orbeckst]

See also #34

[orbeckst]

Help is welcome — with PR #303 we now use loguru everywhere, which has a more modern syntax than logging. Please feel free to add logging where necessary.