Skip to content

Conversation

@syntron
Copy link
Contributor

@syntron syntron commented Apr 25, 2025

Related Issues

logging should be defined in the calling script

reason: the calling script should define how logging is done; if these settings are added here and the caller adds another log handle (with possibly different settings), all log information could be printed twice or not at all

Approach

  • cleanup log definnitions in __ init __.py (first commit)
  • the second commit (re)adds the log definition as a method which could be called if needed

@adeas31
Copy link
Member

adeas31 commented Apr 28, 2025

I think we should define logger and perhaps should do it like this logger = logging.getLogger(__name__). Python logger is a singleton so there shouldn't be an issue of duplicate messages.

We should not configure logger i.e., remove lines like logger.setLevel(logging.DEBUG) and logger.setLevel(logging.WARNING) (btw I don't know why we are first setting the level to debug and then to warning, weird code).

In principle, we should only define and then log messages. Then its up to the user to configure and consume log messages.

I am not sure how we can fix StreamHandler, this is perhaps needed to log messages that comes from omc stream output in some cases.

@ondras12345
Copy link
Contributor

I think we should define logger and perhaps should do it like this logger = logging.getLogger(__name__)

I believe the individual modules already do this. E.g. ModelicaSystem:

logger = logging.getLogger(__name__)

As I understand it, this PR only removes the configuration - that should indeed be left to the user.

@adeas31
Copy link
Member

adeas31 commented Apr 28, 2025

I think we should define logger and perhaps should do it like this logger = logging.getLogger(__name__)

I believe the individual modules already do this. E.g. ModelicaSystem:

logger = logging.getLogger(__name__)

As I understand it, this PR only removes the configuration - that should indeed be left to the user.

Yes. And we should do the same in __init__.py. Also remove the multiple setLevel calls.

And as I said earlier, not sure how to manage stream handler.

reason: the calling script should define how logging is done; if these
settings are added here and the caller adds another log handle (with
possibly different settings), all log information could be printed twice
or not at all
@syntron
Copy link
Contributor Author

syntron commented Apr 28, 2025

updated to latest HEAD and removal of the second part - now it is a clean __ init __.py

@adeas31 similar to the basic logging, StreamHandler could be defined by the user if needed

@adeas31 adeas31 merged commit f6ac6cb into OpenModelica:master Apr 28, 2025
5 checks passed
@syntron syntron deleted the remove_logging branch April 29, 2025 16:52
@syntron syntron mentioned this pull request Sep 28, 2025
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.

3 participants