dismiss "Black is not installed, parameters wont be formatted" message

[edublancas]

papermill shows a logging message if black is not installed, which is causing confusion for ploomber users.

https://github.com/nteract/papermill/blob/aecd0ac16d34dd41a1a8e4ca3351844932af3d94/papermill/translators.py#L195

we should suppress the message

[rishav-karanjit]

Hello @edublancas, you want to remove this message or replace it by something else?

[edublancas]

we want to suppress it, note that this is coming from an external library (papermill), so I think we need to modify the command-line interface code to suppress warnings coming from papermill

to reproduce:

first, get an example

pip install ploomber
ploomber -n templates/ml-basic -o example
cd example
pip install -r requirements.txt

Then:

(ploomber)  Edu@MBP test/ml » ploomber nb -i                                     127 ↵
Loading pipeline...
Black is not installed, parameters wont be formatted
Black is not installed, parameters wont be formatted
Black is not installed, parameters wont be formatted
Black is not installed, parameters wont be formatted
Black is not installed, parameters wont be formatted
Black is not installed, parameters wont be formatted

Note: ensure black isn't installed in the environment

[rishav-karanjit]

Looks good for me.

[edublancas]

maybe you have black installed?

black --help

[rishav-karanjit]

Will look into it @edublancas. Meanwhile, while looking into this issue I found another issue #664

[edublancas]

sure, merged your PR, thanks!

as a tip, I think we want to use the logging module to suppress warnings coming from papermill.

when calling parametrize_notebook, we should temporarily suppress loggings coming from papermill, here's we were are calling parametrize_notebook (note that we are calling it in multiple places):

ploomber/src/ploomber/sources/notebooksource.py

Line 37 in 05e4af0

from papermill.parameterize import parameterize_notebook

[idomic]

@rishav-karanjit any updates here?

[rishav-karanjit]

I don't think I would be working on this issue @idomic

[94rain]

For the reproduce example, I think pip install -r requirements.txt will actually install black for us. I need to run pip uninstall black to reproduce the warning.

Looking into how to suppress warnings from imported modules with logging.

[edublancas]

hi @94rain, the message is coming from papermill, a library we use to execute notebooks. Ideally, we only want to suppress warnings when this message is displayed, as there might be other warnings that are important for the user. the first step is to figure out where exactly papermill is generating the warning. it should be in one of these two places:

when we prepare the notebook:

https://github.com/ploomber/ploomber/blob/master/src/ploomber/sources/notebooksource.py

or when we execute it:

https://github.com/ploomber/ploomber/blob/master/src/ploomber/tasks/notebook.py

[94rain]

I am able to suppress all messages output by the codify function (which outputs the Black is not installed message) using the following code snippet in notebooksource.py.

+   logging.basicConfig(level=logging.ERROR)
     from papermill.parameterize import parameterize_notebook
+    class IgnoreBlackWarning(logging.Filter):
+        def filter(self, record):
+            return not record.funcName == 'codify'
+    logging.getLogger("papermill").addHandler(IgnoreBlackWarning)

I am not sure if there are simpler ways to implement this. I tried to do things likelogging.getLogger("papermill. codify").addHandler(logging.NullHandler()) (or replace papermill with other module names) but does not seem to work.

I'm not sure if adding logging.basicConfig(level=logging.ERROR) there will have some unexpected consequences, and I am still looking into it. (I think we need to pass in the arguments of --log from cli to here as well)

[idomic]

@94rain does it suppress all warnings or just black? I think the better option instead of instantiating a logger is to catch the specific black warning.
Changing the logger level will definitely remove all warnings so we shouldn't do something like that.

Did you try something along the lines of the functions Eduardo sent?

with warnings.catch_warnings():
    warnings.simplefilter('ignore', FutureWarning)
    from papermill.parameterize import parameterize_notebook

Then in it you can match the warning text and remove it only if there's a match.

[94rain]

At this time the snippet above will suppress all warnings. I mean we can pass in the logger level from cli?

ploomber/src/ploomber/cli/parsers.py

Line 513 in 1048e5e

logging.basicConfig(level=args.log.upper())

Yeah, I tried things in that block like the following but they do not seem to work

    warnings.filterwarnings(action='ignore', message=r'.*Black.*')
    warnings.filterwarnings(action='ignore', message='Black is not installed, parameters wont be formatted.')
    warnings.filterwarnings(action='ignore', module=r'.*papermill.*translators.*')

So we have to use the logging module (The warning is generated by logging). I am still finding out if there are easy ways to do text matching.

Another possible approach is to suppress all warnings from the logger of papermill before calling parameterize_notebook and restore it after that

ploomber/src/ploomber/sources/notebooksource.py

Line 868 in 1048e5e

model['content'] = parameterize_notebook(nb,

[idomic]

You're right, for some reason even when putting in the main class:

import warnings
warnings.filterwarnings("ignore")

Which should ignore all warnings, this is still being printed, not sure exactly why.
We can use the logging module to fix this specific one and later explore what's going on.
You can set it as part of the initial code block where we import the papermill function
@edublancas thoughts?

[edublancas]

@94rain is right: papermill is generating the message using logging, so using warnings.filterwarnings has no effect. So the question becomes: how do we use logging to suppress the message.

[94rain]

I am able to suppress all messages output by the codify function (which outputs the Black is not installed message) using the following code snippet in notebooksource.py.

+   logging.basicConfig(level=logging.ERROR)
     from papermill.parameterize import parameterize_notebook
+    class IgnoreBlackWarning(logging.Filter):
+        def filter(self, record):
+            return not record.funcName == 'codify'
+    logging.getLogger("papermill").addHandler(IgnoreBlackWarning)

I am not sure if there are simpler ways to implement this. I tried to do things likelogging.getLogger("papermill. codify").addHandler(logging.NullHandler()) (or replace papermill with other module names) but does not seem to work.

I'm not sure if adding logging.basicConfig(level=logging.ERROR) there will have some unexpected consequences, and I am still looking into it. (I think we need to pass in the arguments of --log from cli to here as well)

After some retries, I just realized what I did wrong here. I need to use addHandler(IgnoreBlackWarning()) (with constructor) instead of addHandler(IgnoreBlackWarning), so it is no longer needed to set logger level.

The following code snippet is able to remove only the messages that contain Black is not installed.

with warnings.catch_warnings():
    warnings.simplefilter('ignore', FutureWarning)
    class IgnoreBlackWarning(logging.Filter):
        def filter(self, record):
            return 'Black is not installed' not in record.msg
    logging.getLogger("papermill.translators").addFilter(IgnoreBlackWarning())

I will create a PR tomorrow. Do we need test cases for this?

[edublancas]

great, thanks for figuring this out. no need to add test cases

question, do we need this?

with warnings.catch_warnings():

I thought we didn't need to use the warnings module, maybe this alone will work?

    class IgnoreBlackWarning(logging.Filter):
        def filter(self, record):
            return 'Black is not installed' not in record.msg
    logging.getLogger("papermill.translators").addFilter(IgnoreBlackWarning())

[94rain]

That alone will work for suppressing the black warning.

I found there is a comment above papermill is importing a deprecated module from pyarrow. I don't know much about the context of this. Looks like it is used to suppress a deprecation warning. Is it still showing up? How can this warning be reproduced?

[edublancas]

ok, so let's just keep the code to remove the black warning and remove the catch_warnings

about pyarrow: they fixed that in the latest papermill release, so no need to worry about it.

[94rain]

Sounds good! Will remove with warnings.catch_warnings(): and create a PR soon.

[idomic]

No worries, thanks for helping out!

…

On Mon, May 23, 2022, 20:09 Rishav karanjit ***@***.***> wrote: I don't think I would be working on this issue @idomic <https://github.com/idomic> — Reply to this email directly, view it on GitHub <#661 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACYPJOOXLEAFKKFHZ5ZEIETVLQM2RANCNFSM5RDLZPJQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>