Restore verbose kwarg to load_xdf

[cboulay]

Fixes #41

[tstenner]

I'm not a fan of those if verbose: log.debug() calls. What about setting the default to None and if it's set changing the log level at the beginning and restoring it at the end?

[cbrnr]

+100 for doing it like @tstenner suggested. I'd implement the following behavior:

• If verbose=None (should be the default), we don't change the logging level and use whatever is currently set by the user.
• If verbose=True, we set the logging level to INFO.
• If verbose=False, we set the logging level to WARNING.
• In addition, string arguments are also possible, e.g. verbose=DEBUG' sets the logging level to DEBUG. These string arguments are equivalent to passing logging.DEBUG` etc.

MNE does it like that, see https://www.nmr.mgh.harvard.edu/mne/stable/generated/mne.set_log_level.html.

[cboulay]

I find it strange that INFO, which is technically above DEBUG, also includes DEBUG. Or am I getting that wrong?
Anyway, I force pushed the suggested changes. I think this is correct.

[cbrnr]

No, INFO does not include DEBUG. It's the other way round, DEBUG includes everything else.

[cboulay]

No, INFO does not include DEBUG. It's the other way round, DEBUG includes everything else.

That's what I guessed from reading about logging levels in the docs though it was never explicit, but then I read something that stated the opposite explicitly. So, if DEBUG is the lowest-level, then why did you recommend that when verbose is provided we set it to INFO instead of DEBUG? Is it just a case of "that's common practice"?

I think we should store the current log level and restore it at the end of the function.

Why? What's the use case? This logger is for this module only.

As far as I can tell, you're trying to prevent a situation where someone first calls load_xdf(verbose=False), then later call load_xdf(verbose=None) expecting the logger to use the root logging level, not the logging level that was set from the previous call.

If someone ever complains about that then by all means you can make a decorator. Decorate your heart out.

[cbrnr]

That's what I guessed from reading about logging levels in the docs though it was never explicit, but then I read something that stated the opposite explicitly. So, if DEBUG is the lowest-level, then why did you recommend that when verbose is provided we set it to INFO instead of DEBUG? Is it just a case of "that's common practice"?

Yep.

Why? What's the use case? This logger is for this module only.

As far as I can tell, you're trying to prevent a situation where someone first calls load_xdf(verbose=False), then later call load_xdf(verbose=None) expecting the logger to use the root logging level, not the logging level that was set from the previous call.

If someone ever complains about that then by all means you can make a decorator. Decorate your heart out.

Fair enough.

[cbrnr]

Feel free to change the behavior of verbose=True to setting the log level to DEBUG. I think that's what @chkothe had in mind in the first place. Other than that, +1 for merge.

[cboulay]

OK back to DEBUG. I'll run a couple more tests locally then merge.