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

Replace verbose=true print statements with logging #193

Open
adamsardar opened this issue Mar 6, 2024 · 0 comments
Open

Replace verbose=true print statements with logging #193

adamsardar opened this issue Mar 6, 2024 · 0 comments
Labels
good first issue Good for newcomers

Comments

@adamsardar
Copy link
Contributor

adamsardar commented Mar 6, 2024

What

Rather than rely on a verbose flag to dictate whether we should print output to stdout, it would be better to rely on standard logging tools and severity warning levels to control what is directed where. It would also offer greater granularity over alerts: INFO, DEBUG, WARNING, ERROR and CRITICAL. When troubleshooting a user could enable INFO level,but in prod just WARNING (and above).

Why

From the core Python docs, the advice is:

If you want to:

Display console output for ordinary usage of a command line script or program

Use print(msg).

But if you want to

Report events that occur during normal operation of a program (e.g. for status monitoring or fault investigation)

Use logging.info() or logging.debug(). There's an analogous statement for warnings and logging.warning().

How

Here is an example:

if self.verbose:
    print("BaseTransformer.fit() called")

Which I would suggest replacing with:

import logging
logging.info('BaseTransformer.fit() called')

At runtime, a user can opt-in to seeing these log traces:

logging.basicConfig(level=logging.DEBUG)
logging.debug('this will be displayed')
logging.info('this will not because INFO is below DEBUG')

The task of this ticker is therefore to find all places in the code that uses print to report application state to the user/process and replace it with logging. This would leave the verbose redundant and allow for its removal, simplifying the API - delete their use. This may break some implementation tests (see #155), in which case consider their removal if they are testing function call arguments (but not if they are testing behaviour of the code).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant