Add T20 ruff rule (print calls)

[brisvag]

References and relevant issues

Tracking issue: #5589

Previous discussion: #6840

Description

As asked in #6840, I split out this more controversial rule from it, so we can discuss it separately.

[brisvag]

@jni, I switched to logging where I thought it made sense, keeping print + noqa only for the reasonable cases. Still not sure this is worth it, but at least it doesn't introduce big changes.

[brisvag]

@jni input appreciated!

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 48.00000% with 13 lines in your changes are missing coverage. Please review.

Project coverage is 92.43%. Comparing base (a9c25e6) to head (7478720). Report is 5 commits behind head on main.

Files	Patch %	Lines
napari/__main__.py	50.00%	2 Missing :warning:
napari/_qt/dialogs/qt_notification.py	0.00%	2 Missing :warning:
napari/_qt/widgets/qt_theme_sample.py	0.00%	2 Missing :warning:
napari/components/experimental/monitor/_monitor.py	0.00%	2 Missing :warning:
napari/utils/perf/_patcher.py	60.00%	2 Missing :warning:
napari/utils/notifications.py	66.66%	1 Missing :warning:
napari/utils/perf/_timers.py	0.00%	1 Missing :warning:
napari/utils/stubgen.py	0.00%	1 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #6849 +/- ## ========================================== - Coverage 92.45% 92.43% -0.02% ========================================== Files 614 613 -1 Lines 55166 55153 -13 ========================================== - Hits 51002 50983 -19 - Misses 4164 4170 +6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[jni]

This PR changes 11 prints to print # noqa, and 7 prints to logging/warn. (And note that for our purposes logging is in fact equivalent to print.) imho, that is a bit of a smell that the rule is too stringent and we should not activate it. I'm not actively going to block it but yeah, I don't love it.

[Czaki]

(And note that for our purposes logging is in fact equivalent to print.)

And we should change this. We should implement a custom logger grabber and GUI to inspect logs from application.

[jni]

And we should change this. We should implement a custom logger grabber and GUI to inspect logs from application.

Ok I'm convinced. Maybe changing the remaining prints to logs then makes sense.

[brisvag]

This PR changes 11 prints to print # noqa, and 7 prints to logging/warn.

It's more like "at least half of our prints shouldn't be there" :P Also, this rule is more to prevent accidentally adding prints to places where they shouldn't be; the few prints that should be there (e.b the outputs of the cli) are already there and are unlikely to change.

Agree it's not a super-important rule, but I think it does more good than bad. Agree we should have a log handler we grab everywhere! It could be done as part of this PR as well. @Czaki I'm not sure I follow the GUI comment; can you elaborate?

[Czaki]

Agree it's not a super-important rule, but I think it does more good than bad. Agree we should have a log handler we grab everywhere! It could be done as part of this PR as well. @Czaki I'm not sure I follow the GUI comment; can you elaborate?

Something like icy Output tab:

Or console log in ImageJ. So place where it will be accessible to user who launch napari without a terminal.

It may be even smarter (like runtime level filtering, or selection of which logger output should be published).

[jni]

I don't think the logging console needs to be part of this PR (indeed, maybe not even this repo 😂), but rather, change every remaining print to a log. What do you think @brisvag @Czaki ?

[Czaki]

I don't think the logging console needs to be part of this PR (indeed, maybe not even this repo 😂)

I think it should be part of the default installation napari[all], but maybe outside this repo. But if @brisvag want, it could be even part of this PR.

but rather, change every remaining print to a log.

No. Output of napari --info should be printed using print, not logger.

[brisvag]

I think it should be part of the default installation napari[all], but maybe outside this repo. But if @brisvag want, it could be even part of this PR.

Definitely not this PR, but I agree that it would be nice to just have it somewhere. As long as we use logging properly, we should be able to do this at a later stage.

No. Output of napari --info should be printed using print, not logger.

Yeah, and a couple other things!

@Czaki: what's the general idea behing using warnings.warn vs logging.warning? We seem to use them randomly, maybe we should always use log?

[Czaki]

@Czaki: what's the general idea behing using warnings.warn vs logging.warning? We seem to use them randomly, maybe we should always use log?

warnings are shown as modal dialog in the bottom-right corner and logging is not displayed.

I think that warning is something that should be immediately displayed to the user and logging for later access.

[brisvag]

But don't we want the warnings to the user to also be displayed in the logs? Wouldn't it be cleaner to take all logs from level WARN or more and push them as notifications, instead of using two different systems? I also just confused about why python offers these two systems in the standard library...

[Czaki]

For example, warning by default provides deduplication of events (with well, known bug). And logging push to logs every event that happens.

Warnings were introduced in python 2.1 and logging in python 2.3 (based on documentation).

Warnings are global, logging offers the option to have multiple loggers with different settings.

I see that there are similar, but not equivalent. I'm not sure if we want to change every logging into notification.

[brisvag]

I changed 1 more print into log, but I think the rest should remain noqa because they are all special cases. Let me know if you disagree!

I also moved the import time to a benchmark based on the implementation at https://github.com/proost/pandas/blob/master/asv_bench/benchmarks/package.py. Let's see if it works :)

[github-actions[bot]]

The Qt benchmark run requested by PR #6849 (09eb7e3454021dbdf37e7b870c84acb722804f4e vs 1dcbadea20149c43d49764afd3fe8b39b76998a9) has finished with status 'failure'. See the CI logs and artifacts for further details. The non-Qt benchmark run requested by PR #6849 (09eb7e3454021dbdf37e7b870c84acb722804f4e vs 1dcbadea20149c43d49764afd3fe8b39b76998a9) has finished with status 'success'. See the CI logs and artifacts for further details.

[github-actions[bot]]

The Qt benchmark run requested by PR #6849 (cd2d5da1699440e8ed5a9bbe338357cc85ef16e5 vs 1dcbadea20149c43d49764afd3fe8b39b76998a9) has finished with status 'success'. See the CI logs and artifacts for further details. The non-Qt benchmark run requested by PR #6849 (cd2d5da1699440e8ed5a9bbe338357cc85ef16e5 vs 1dcbadea20149c43d49764afd3fe8b39b76998a9) has finished with status 'success'. See the CI logs and artifacts for further details.

[Czaki]

@brisvag It looks like the last merge to main introduced merge conflict