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

Log to the console rather than print() #235

Closed
lochhh opened this issue Jul 10, 2024 · 5 comments · Fixed by #209
Closed

Log to the console rather than print() #235

lochhh opened this issue Jul 10, 2024 · 5 comments · Fixed by #209
Labels
question Further information is requested

Comments

@lochhh
Copy link
Collaborator

lochhh commented Jul 10, 2024

I assume @VascoSch92 meant that this should be logged to the console rather than printed (e.g. so verbosity can be changed by third parties).

Originally posted by @adamltyson in #209 (comment)

When reporting number of NaNs in filtering, we currently use a print statement to print this to the console, whilst also logging this to the log attribute of a DataArray.

@lochhh lochhh added the question Further information is requested label Jul 10, 2024
@adamltyson
Copy link
Member

In general (but not necessarily absolutely) I think movement should avoid printing things to the console. This could get annoying if a user has a script that repeats a process many times. It should be possible for the user to configure what gets printed using the logging module.

@niksirbi
Copy link
Member

I agree on this principle.

In this case, it's very useful to quickly see the number of NaNs before/after an operation. Given the new utility functions that @lochhh has nicely refactored into the utils/reports.py module, what would be our recommended way for viewing the report (e.g. in a notebook cell)?

One way would be for report_nan_values() to also return the report as a string, so one can do print(report_nan_values(ds)). Or are there easier ways of doing this by configuring the logger to show INFO in the cell output?

@adamltyson
Copy link
Member

I think I wouldn't expect to use logging to see this in a notebook necessarily, but an explicit call, so something like print(report_nan_values(ds))

@niksirbi
Copy link
Member

I agree, so perhaps we should modify that function in #209 to return the string?

@lochhh lochhh linked a pull request Jul 11, 2024 that will close this issue
10 tasks
@niksirbi
Copy link
Member

Thanks @lochhh ! We can close this issue once I approve #209 and we merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants