Skip to content

ETNA-773: Adding ConsoleLogger #46

Merged
merged 43 commits into from Sep 21, 2021
Merged

ETNA-773: Adding ConsoleLogger #46

merged 43 commits into from Sep 21, 2021

Conversation

Mr-Geekman
Copy link
Contributor

Add ConsoleLogger.

@Mr-Geekman Mr-Geekman added the enhancement New feature or request label Sep 14, 2021
@Mr-Geekman Mr-Geekman self-assigned this Sep 14, 2021
@codecov-commenter
Copy link

codecov-commenter commented Sep 14, 2021

Codecov Report

Merging #46 (f814e36) into master (61aba21) will decrease coverage by 4.26%.
The diff coverage is 94.06%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #46      +/-   ##
==========================================
- Coverage   90.48%   86.22%   -4.27%     
==========================================
  Files          45       48       +3     
  Lines        1976     2127     +151     
==========================================
+ Hits         1788     1834      +46     
- Misses        188      293     +105     
Impacted Files Coverage Δ
etna/datasets/tsdataset.py 86.77% <66.66%> (-0.33%) ⬇️
etna/loggers/base.py 92.30% <92.30%> (ø)
etna/loggers/console_logger.py 92.59% <92.59%> (ø)
etna/loggers/__init__.py 100.00% <100.00%> (ø)
etna/model_selection/backtest.py 97.56% <100.00%> (+0.14%) ⬆️
etna/models/base.py 97.40% <100.00%> (+0.52%) ⬆️
etna/models/catboost.py 100.00% <100.00%> (ø)
etna/models/nn/deepar.py 45.94% <100.00%> (-54.06%) ⬇️
etna/models/nn/tft.py 43.58% <100.00%> (-56.42%) ⬇️
etna/models/sklearn.py 100.00% <100.00%> (ø)
... and 9 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 61aba21...f814e36. Read the comment docs.

from etna.core.mixins import BaseMixin


class Logger(ABC, BaseMixin):
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe Logger -> BaseLogger (AbstractLogger)?
To explicit point that it is abstract class

Copy link
Contributor

@martins0n martins0n left a comment

Choose a reason for hiding this comment

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

Some models left without wrapper -- nn models, for example

pass


class LoggerComposite(BaseLogger):
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's make private _Logger.

class LoggerComposite(BaseLogger):
"""Composite for loggers."""

def __init__(self, *args):
Copy link
Contributor

@martins0n martins0n Sep 17, 2021

Choose a reason for hiding this comment

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

We don't need such difficult init now, because we initialize it once.

from typing import Union

import pandas as pd
from loguru import logger
Copy link
Contributor

Choose a reason for hiding this comment

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

lets import as _logger

super().__init__()
if 0 in logger._core.handlers:
logger.remove(0)
logger.add(sink=sys.stderr)
Copy link
Contributor

@martins0n martins0n Sep 17, 2021

Choose a reason for hiding this comment

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

We should make:
self.logger = _logger.opt(depth=1, lazy=True, colors=True)
(lets try at least -- may be some problems with pickling will arise)

"""
pass

def set_config(self, forecaster):
Copy link
Contributor

@martins0n martins0n Sep 17, 2021

Choose a reason for hiding this comment

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

We don't need this now.

@martins0n martins0n self-requested a review September 21, 2021 12:38
Copy link
Contributor

@martins0n martins0n left a comment

Choose a reason for hiding this comment

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

👍

@martins0n martins0n merged commit 6c12eb6 into master Sep 21, 2021
@martins0n martins0n deleted the ETNA-773 branch September 21, 2021 13:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants