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

Remove the warnings upfront when importing the light-weight versions #927

Closed
aaronsl-hku opened this issue Apr 21, 2022 · 5 comments · Fixed by #1010
Closed

Remove the warnings upfront when importing the light-weight versions #927

aaronsl-hku opened this issue Apr 21, 2022 · 5 comments · Fixed by #1010
Assignees
Labels
good first issue Good for newcomers improvement New feature or improvement
Projects

Comments

@aaronsl-hku
Copy link

Is your feature request related to a current problem? Please describe.
Yes. Currently using the "u8darts[torch]" version (for all the chaos for installing prophet but that's another story), every time I run my program with darts imported, I would received the warnings from darts.models.init.py.

It would not be a problem if the logger is instantiated directly from logging module, but with levels, format and addHandler hardcoded in darts.logging.get_logger() there's basically nothing I can do.

Describe proposed solution
The best solution would be completely replace the darts.logging.get_logger() function with standard logging.getLogger() (unless there're any very special reasons).

Describe potential alternatives

  • Do not add handlers other than NullHandler as mentioned here
  • Raise the warnings in a lazy manner, if that is plausible.
@aaronsl-hku aaronsl-hku added the triage Issue waiting for triaging label Apr 21, 2022
@hrzn hrzn added this to To do in darts via automation May 22, 2022
@hrzn hrzn added good first issue Good for newcomers improvement New feature or improvement and removed triage Issue waiting for triaging labels May 22, 2022
@brunnedu brunnedu moved this from To do to In progress in darts Jun 16, 2022
@brunnedu brunnedu self-assigned this Jun 16, 2022
@brunnedu
Copy link
Contributor

brunnedu commented Jun 16, 2022

Hi @aaronsl-hku, we would really like to help you resolve this issue. At the same time, we think that it is important to inform our users about missing modules.

Therefore we would suggest giving you the option to opt-out of those warnings. Would it be possible for you to set an environment variable such as DISABLE_DARTS_LOGGING in your terminal? We could then use this environment variable to adjust the logging behavior of darts.

@hrzn
Copy link
Contributor

hrzn commented Jun 16, 2022

@martinb-bb WDYT of this solution?

@brunnedu brunnedu moved this from In progress to In review in darts Jun 16, 2022
@aaronsl-hku
Copy link
Author

Hello @brunnedu . Nice to receive your reply.

We might have some misunderstanding here: I am not looking into disable all warnings as a whole. What I am proposing is not to hardcode the handler and formatter into the get_logger function.

Long story short, if my project has a logging config and the formatting is not the same darts, my log would be easily screwed by the hardcoded formatter.

May I know if there is any particular reason not to use the standard logging.get_logger as-is?

Hi @aaronsl-hku, we would really like to help you resolve this issue. At the same time, we think that it is important to inform our users about missing modules.

Therefore we would suggest giving you the option to opt-out of those warnings. Would it be possible for you to set an environment variable such as DISABLE_DARTS_WARNINGS in your terminal? We could then use this environment variable to adjust the logging behavior of darts.

@brunnedu brunnedu moved this from In review to In progress in darts Jun 17, 2022
@martinb-ai
Copy link

martinb-ai commented Jun 17, 2022

@martinb-bb WDYT of this solution?

I think this could work. Being able to handle logging like the traditional python logger would be fine...or even a flag would be great.

@hrzn Please let me know when this change is pushed so I can test on our terminal 😄

@brunnedu
Copy link
Contributor

Thanks for your suggestions, everyone!

I just added another commit: ad7a404

@hrzn hrzn moved this from In progress to In review in darts Jun 21, 2022
@hrzn hrzn linked a pull request Jun 21, 2022 that will close this issue
darts automation moved this from In review to Done Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers improvement New feature or improvement
Projects
darts
Done
Development

Successfully merging a pull request may close this issue.

4 participants