ocrd_cli_wrap_processor: always do initLogging#1296
Conversation
|
(Of course, as already commented in #1223, it may still be necessary to be careful with imports on the side of the processor implementations. For example, moving Alternatively, we could still do |
kba
left a comment
There was a problem hiding this comment.
(Of course, as already commented in #1223, it may still be necessary to be careful with imports on the side of the processor implementations. For example, moving
import tensorflowtosetup.)
I am wondering how future-proof these mechanisms are. There have been different workarounds to prevent tensorflow and others from logging, like our tf_disable_interactive_logs and various copy-and-pasted stanzas like in eynollah:
os.environ["TF_CPP_MIN_LOG_LEVEL"] = "3"
stderr = sys.stderr
sys.stderr = open(os.devnull, "w")
import tensorflow as tf
from tensorflow.python.keras import backend as K
from tensorflow.keras.models import load_model
sys.stderr = stderr
tf.get_logger().setLevel("ERROR")
warnings.filterwarnings("ignore")And apart from being really intrusive and inconsistent, they could break at any point.
Alternatively, we could still do
initLoggingon the module level, though not inocrd_utils.loggingbut ratherocrd.decorators, which arguably will only be imported by applications that do need log handlers.
Module-level in ocrd.decorators.__init__.py makes sense and a cursory glance over how eynollah and ocrd_kraken are structured make it seem this would work. For ocrd_calamari, there is a top-level __init__.py which does from .recognize import CalamariRecognize, i.e. tensorflow is imported before ocrd.decorators but it is guarded by tf_disable_interactive_logs.
So, as much as I dislike module-level function calls in general, in the interest of fewer surprises in the future, I'm for putting initLogging in ocrd.decorators.__init__.
right, but that just covers the rogue
That should not be necessary if we run our
wow, that's rather extreme, it speaks to the frustration that we had with correct logging setup in the Python ecosystem
should also not be necessary anymore
That's something we could indeed add into |
|
Note to self: we still need that as part of v3 / |
fixes #1223 and replaces #1295