astrogilda / tsbootstrap

tsbootstrap: generate bootstrapped time series samples in Python
https://tsbootstrap.readthedocs.io/en/latest/
MIT License
65 stars 5 forks source link

Replace Print Statements with Logging #116

Closed benHeid closed 3 months ago

benHeid commented 3 months ago

Currently, there are a lot of print statements that prints outputs into the console. Due to the usage of print, it is not controllable what is printed. Thus, I would propose to use logging or loguru to enable more control here.

Thereby, it should be paid attention on setting an appropriate logging level. I suppose most of the prints should be log level debug.

from sktime.transformations.bootstrap import TSBootstrapAdapter

from sktime.datasets import load_airline
from sktime.forecasting.model_selection import temporal_train_test_split

data = load_airline()
train, test = temporal_train_test_split(data, test_size=24)

adapter = TSBootstrapAdapter(BartlettsBootstrap())

result = adapter.fit_transform(test)
image
fkiraly commented 3 months ago

hm, these are not too informative to the user imo?

For informative statements, the standard is to introduce a verbose: bool argument, and only print if verbose=True.

astrogilda commented 3 months ago

I agree with Franz

benHeid commented 3 months ago

Since they are not that informative, I used logging.debug. We can also couple the verbose statement with the logging by changing the log level depending on the set verbose level or flag.

astrogilda commented 3 months ago

Year I'd recommend this approach.

On Mon, Apr 1, 2024, 4:01 AM Benedikt Heidrich @.***> wrote:

Since they are not that informative, I used logging.debug. We can also couple the verbose statement with the logging by changing the log level depending on the set verbose level or flag.

— Reply to this email directly, view it on GitHub https://github.com/astrogilda/tsbootstrap/issues/116#issuecomment-2029348885, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFTOOHSTT4QS5RDRNPYJUSLY3EH47AVCNFSM6AAAAABFOQEYH2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRZGM2DQOBYGU . You are receiving this because you commented.Message ID: @.***>

astrogilda commented 3 months ago

Year I'd recommend this approach. On Mon, Apr 1, 2024, 4:01 AM Benedikt Heidrich @.> wrote: Since they are not that informative, I used logging.debug. We can also couple the verbose statement with the logging by changing the log level depending on the set verbose level or flag. — Reply to this email directly, view it on GitHub <#116 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFTOOHSTT4QS5RDRNPYJUSLY3EH47AVCNFSM6AAAAABFOQEYH2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRZGM2DQOBYGU . You are receiving this because you commented.Message ID: @.>

@benHeid could you let us know when the PR is ready to be reviewed?