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

5959 uses logging config at module level #5960

Merged
merged 1 commit into from
Feb 8, 2023
Merged

Conversation

wyli
Copy link
Contributor

@wyli wyli commented Feb 8, 2023

Signed-off-by: Wenqi Li wenqil@nvidia.com

Fixes #5959

Description

avoid modifying the system logging config

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Copy link
Collaborator

@holgerroth holgerroth left a comment

Choose a reason for hiding this comment

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

Good change!

@wyli
Copy link
Contributor Author

wyli commented Feb 8, 2023

/build

@wyli wyli enabled auto-merge (squash) February 8, 2023 21:30
@wyli wyli merged commit 82cd668 into Project-MONAI:dev Feb 8, 2023
@wyli wyli deleted the logging-issue branch February 8, 2023 22:13
@wyli wyli mentioned this pull request Feb 22, 2023
7 tasks
wyli added a commit that referenced this pull request Feb 25, 2023
### Description

The recommendation in `StatsHandler` of setting the global logging
config may not work in some cases (e.g. google colab and
#5960)
https://docs.python.org/3/library/logging.html#logging.basicConfig


https://github.com/Project-MONAI/MONAI/blob/3eef61e82883e9bc6c08d26954fc86286df7d1b3/monai/handlers/stats_handler.py#L55

in general we shouldn't recommend changing the root logger directly.

This PR changes the message to tune the module level loggers, updated
example message will be:
```
.the effective log level of ignite.engine.engine.Engine higher than INFO, StatsHandler may not generate logs,
please use the following code before running the engine to enable it: 
                                                                                
import logging                                                              
logging.getLogger('ignite.engine.engine.Engine').setLevel(logging.INFO) 
```

the updated commands are tested in the colab example:

https://colab.research.google.com/drive/1boqy7ENpKrqaJoxFlbHIBnIODAs1Ih1T

### Types of changes
<!--- Put an `x` in all the boxes that apply, and remove the not
applicable items -->
- [x] Non-breaking change (fix or new feature that would not break
existing functionality).
- [ ] Breaking change (fix or new feature that would cause existing
functionality to change).
- [x] New tests added to cover the changes.
- [ ] Integration tests passed locally by running `./runtests.sh -f -u
--net --coverage`.
- [x] Quick tests passed locally by running `./runtests.sh --quick
--unittests --disttests`.
- [x] In-line docstrings updated.
- [ ] Documentation updated, tested `make html` command in the `docs/`
folder.

---------

Signed-off-by: Wenqi Li <wenqil@nvidia.com>
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.

Monai import modifies logger
2 participants