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

Opt-out of darts logging based on environment variable #1010

Merged
merged 3 commits into from
Jun 21, 2022

Conversation

brunnedu
Copy link
Contributor

@brunnedu brunnedu commented Jun 16, 2022

OUTDATED, SEE BELOW FOR UPDATE

Addresses #927

Summary

  • Implemented option to opt-out of darts logging based on the environment variable DISABLE_DARTS_LOGGING
  • If DISABLE_DARTS_LOGGING is set to the string "1", the StreamHandler is replaced with the NullHandler in the darts.logging.get_logger() function.

Other Information

I'm open to any suggestions on how we could handle this better.

@codecov-commenter
Copy link

codecov-commenter commented Jun 16, 2022

Codecov Report

Merging #1010 (ad7a404) into master (abf12da) will increase coverage by 0.11%.
The diff coverage is 85.18%.

@@            Coverage Diff             @@
##           master    #1010      +/-   ##
==========================================
+ Coverage   92.92%   93.03%   +0.11%     
==========================================
  Files          76       77       +1     
  Lines        7628     7857     +229     
==========================================
+ Hits         7088     7310     +222     
- Misses        540      547       +7     
Impacted Files Coverage Δ
darts/logging.py 98.18% <ø> (-0.13%) ⬇️
darts/models/__init__.py 81.39% <0.00%> (ø)
darts/models/forecasting/block_rnn_model.py 98.14% <ø> (ø)
darts/models/forecasting/nbeats.py 98.07% <ø> (+0.01%) ⬆️
darts/models/forecasting/nhits.py 99.25% <ø> (+<0.01%) ⬆️
darts/models/forecasting/rnn_model.py 97.46% <ø> (ø)
darts/models/forecasting/tcn_model.py 96.87% <ø> (+0.03%) ⬆️
darts/models/forecasting/tft_model.py 96.92% <ø> (ø)
darts/models/forecasting/transformer_model.py 100.00% <ø> (ø)
darts/models/forecasting/pl_forecasting_module.py 93.79% <100.00%> (+0.84%) ⬆️
... and 23 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c43352...ad7a404. Read the comment docs.

@brunnedu
Copy link
Contributor Author

Addresses #927

Summary

After some helpful suggestions from the community, we have now decided that the best solution is to stop adding custom handlers or formatters to our loggers.

  • darts.logging.get_logger(name) is now a simple wrapper of logging.getLogger(name)
  • darts.logging.get_logger() sets the level of newly created loggers to INFO which is the default logging level of Darts

@brunnedu brunnedu marked this pull request as ready for review June 21, 2022 14:00
Copy link
Contributor

@hrzn hrzn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@hrzn hrzn linked an issue Jun 21, 2022 that may be closed by this pull request
@martinb-ai
Copy link

martinb-ai commented Jun 21, 2022

@brunnedu Just following along both threads, how do you want us to use the new wrapper to ignore the warnings of non-installed packages? Stick to only keeping logging level to warnings?

# import and set logging level
import logging
logging.basicConfig(level=logging.WARNING)
# only then import darts 
import darts

?

@brunnedu
Copy link
Contributor Author

brunnedu commented Jun 23, 2022

@martinb-bb Unfortunately using logging.basicConfig() wouldn't work, because after creating a new logger in get_logger() we set its level to INFO overwriting whatever level was set in logging.basicConfig(). Because of that we now decided to not use logger.setLevel(logging.INFO) anymore (#1034) which would then allow you to disable the warnings using:

import logging
# set level to ERROR, not logging levels WARNING and below anymore
logging.basicConfig(level=logging.ERROR)
import darts
from darts.models import RegressionModel

Another option that works in either case would be to use:

import contextlib
import os
with open(os.devnull,'w') as handle:
    with contextlib.redirect_stderr(handle):
        import darts
        from darts.models import RegressionModel

This could be especially useful if you don't want to modify logging.basicConfig and just want to disable warnings for a few import lines

@madtoinou madtoinou deleted the feat/remove_model_warnings branch July 5, 2023 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove the warnings upfront when importing the light-weight versions
4 participants