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

Prettify stdout #755

Open
hasan7n opened this issue Dec 10, 2023 · 11 comments · May be fixed by #842
Open

Prettify stdout #755

hasan7n opened this issue Dec 10, 2023 · 11 comments · May be fixed by #842
Assignees
Labels
enhancement New feature or request

Comments

@hasan7n
Copy link
Contributor

hasan7n commented Dec 10, 2023

Is your feature request related to a problem? Please describe.
The STDOUT can be full of warnings, it should be more user friendly

Describe the solution you'd like
Make GaNLDF's stdout more user friendly (how?)

Describe alternatives you've considered
N/A

Additional context
Warnings could be useful debugging. Perhaps we can have an organized logging system: current stdout can be split into stdout for users, and debugging logs in a ~/.gandlf_logs file for example.

@sarthakpati sarthakpati added the help wanted Extra attention is needed label Dec 11, 2023
@sarthakpati
Copy link
Collaborator

Great idea! It might make sense to put logs in ${output_dir}/logs/ instead of ~/.gandlf_logs, though. Do you have bandwidth to work on this?

@sylwiamm
Copy link
Contributor

sylwiamm commented Jan 4, 2024

@sarthakpati Assign this to me

@sarthakpati sarthakpati added enhancement New feature or request and removed help wanted Extra attention is needed labels Feb 13, 2024
@sylwiamm sylwiamm linked a pull request Apr 3, 2024 that will close this issue
10 tasks
Copy link
Contributor

Stale issue message

@sarthakpati
Copy link
Collaborator

I believe @sylwiamm is still working on this.

@benmalef
Copy link
Contributor

benmalef commented Jun 3, 2024

Hi guys, I hope you are well.
I have started to check it. I would like to ask some questions.

  1. What do you think about having a config file in YAML format like this. In this way, it can easily modified by other developers without any extra coding.
  2. As @VukW has already mentioned ref. we can use, for example the getLogger(name). Additionally, a logger with the same name as the module name should exist in the config file. Any thoughts on this ??

@benmalef benmalef mentioned this issue Jun 4, 2024
10 tasks
@sarthakpati
Copy link
Collaborator

Hi @benmalef,

  1. Can you give an example? I am not sure I completely follow this. 😵‍💫
  2. I don't have any strong suggestions/recommendations. Perhaps @VukW can comment, since he has far more experience with this?

@benmalef
Copy link
Contributor

benmalef commented Jun 4, 2024

Hi @benmalef,

  1. Can you give an example? I am not sure I completely follow this. 😵‍💫
  2. I don't have any strong suggestions/recommendations. Perhaps @VukW can comment, since he has far more experience with this?

Hi @sarthakpati,
Here is an example.

The configuration of logging in yaml format can be like this.

version: 1
formatters:
  detailed:
    format: "%(asctime)s - %(name)s - %(levelname)s - %(message)s" 
debugHandler:
    class: logging.FileHandler
    filename: gandlf.log
    formatter: detailed
    level: DEBUG
Loggers:
    debug_logger:
    level: DEBUG
root:
  level: DEBUG
  handlers: [debugHandler]

define a def for reading the config file

def logger_setup( logger_name, config_pat):
    with open(config_path, "r") as file:
         logger_config = yaml.safe_load(file)
          logging.config.dictConfig(logger_config)
  return logging.getLogger(logger_name)

We can use the logger directly in the module with this:

logger =  logger_setup("debug_logger")
logger.info("You are the best")

I hope now is more clear.

If @VukW can provide any comment, it would be very helpful.

@VukW
Copy link
Contributor

VukW commented Jun 6, 2024

Hi folks!

  • What do you think about having a config file in YAML format like this. In this way, it can easily modified by other developers without any extra coding.

I'm always afraid of yaml logging config files, but if you are able to configure it properly - that's fine solution IMO

  • As @VukW has already mentioned ref. we can use, for example the getLogger(name). Additionally, a logger with the same name as the module name should exist in the config file. Any thoughts on this ??

🤔 don't loggers inherit settings from each other? Can we configure just root logger (or some gandlf-main logger), and during logger_setup create a new logger by given name, inherited from this gandlf-main? I mean, declaring each possible logger in the configuration may be error-prone as:

  1. We need to declare/configure a couple of dozens loggers, and in future PRs check that renaming existing loggers in the code align with renaming them in config file
  2. that may lead to different configurations for different loggers (or their config duplication). Of course if some loggers want to use specific configuration they should be declared separately; but most of loggers, I believe, should keep the same configuration

@VukW
Copy link
Contributor

VukW commented Jun 6, 2024

UPD @benmalef I tested your configuration with a logger that is not defined in .yaml. It works well also as it inherits from root logger you've configured. So, no need to explicitly define all possible loggers in yaml config

@benmalef
Copy link
Contributor

benmalef commented Jun 6, 2024

Hi @VukW, I hope you are well.
Thanks for your response.

Hi folks!

  • What do you think about having a config file in YAML format like this. In this way, it can easily modified by other developers without any extra coding.

I'm always afraid of yaml logging config files, but if you are able to configure it properly - that's fine solution IMO

Hmm, okay. I believe it is more user-friendly, but it is the first time I have tried to implement logging. If you have any suggestions or recommendations, they are kindly accepted.

@benmalef
Copy link
Contributor

benmalef commented Jun 6, 2024

UPD @benmalef I tested your configuration with a logger that is not defined in .yaml. It works well also as it inherits from root logger you've configured. So, no need to explicitly define all possible loggers in yaml config

Yes, you are right, I have tested, too. Thanks for your comment :P.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants