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

Use logger instead of print statements in "verbose" mode #175

Open
wtbarnes opened this issue Aug 31, 2023 · 6 comments
Open

Use logger instead of print statements in "verbose" mode #175

wtbarnes opened this issue Aug 31, 2023 · 6 comments
Labels
Code Improvement For general improvements to the codebase without adding new features. discussion For more general issues that require extended discussion
Milestone

Comments

@wtbarnes
Copy link
Collaborator

Description

There are several places throughout the package where print statements are used to inform the user about something that is happening when a "verbose" flag is set. Instead of print statements, it would be better to use a logger throughout the package. This allows a user to more easily control how much is printed to the screen.

@jslavin
Copy link
Contributor

jslavin commented Sep 6, 2023

I'm not sure that I agree that using a logger is easier for the user. Doesn't the logger setting affect all the routines one uses? What if you just want verbose output from one particular function?

@nabobalis
Copy link
Member

I'm not sure that I agree that using a logger is easier for the user. Doesn't the logger setting affect all the routines one uses?
What if you just want verbose output from one particular function?

You can change the logging output level using a context manager or just calling the logger directly before a function call. I can provide code examples if wanted.

Using a logger is the standard Python practice for providing information to a user. So a developer can segment the information provided to the user of a function. This offers a much finer control than a verbose keyword and a print statement.

I would suggest that if you are writing a library in Python, following the best practices (if I personally disagree with some of them) that have been refined over the years for a programming language is the better approach when anyone is creating to library to provide to users. This will require a different mindset that can and will be jarring at times, that is inescapable due to the difference in programming languages.

@jslavin
Copy link
Contributor

jslavin commented Sep 15, 2023

What would you think of using logging within the functions but retaining the verbose keyword? So something like:

def func(..., verbose=False)
    ...
    if verbose:
        logging.basicConfig(level=logging.INFO)

or something like that. I'm just trying to make things simple for users that are using the functions in interactive environments.

@joyvelasquez joyvelasquez added this to the 0.5.0 milestone Nov 22, 2023
@wtbarnes
Copy link
Collaborator Author

wtbarnes commented Sep 3, 2024

Apologies for losing track of this issue.

My opinion is that using a verbose keyword as noted above conflicts with the controlled output that logging offers you. As an example, if I set the log level to INFO but then I don't set verbose=True, then I don't get the expected logging messages. Using verbose also increases code complexity by adding more possible code paths because of the need for a conditional.

I can appreciate this is a change for many users, particularly those coming from IDL. However, I do not think it is too onerous to ask users to set the appropriate logging level in their scripts if they want more verbose feedback. We could use this as an opportunity to educate users on the advantages of logging.

@wtbarnes wtbarnes added Code Improvement For general improvements to the codebase without adding new features. discussion For more general issues that require extended discussion labels Sep 3, 2024
@jslavin
Copy link
Contributor

jslavin commented Sep 3, 2024

So I haven't thought about this issue for a while, but I'm still not clear on how the setting of log level can be limited to a single procedure. For example in the routine temperature_from_filter_ratio I used verbose to tell the routine to output information about how the image was processed. If that was the only INFO level logging that I want, how would I only set the logging level such that I only got that and not INFO level logging for other routines called? Maybe using the context manager as Nabil mentioned. Sitll if I'm working from the command line, say using ipython and I want to plot up the temperature image and get the information about it, it seems much simpler to me to just provide the verbose keyword.
This may be something best discussed in person when you two are here.

@wtbarnes
Copy link
Collaborator Author

wtbarnes commented Sep 3, 2024

I'm not sure I completely understand your use case here, but yes I think using the context manager would be one way. Happy to discuss more tomorrow and then record any consensus here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Improvement For general improvements to the codebase without adding new features. discussion For more general issues that require extended discussion
Projects
None yet
Development

No branches or pull requests

4 participants