Skip to content

Use standard stream for loggers#1601

Merged
vfdev-5 merged 12 commits intopytorch:masterfrom
sdesrozis:logger_stream
Feb 2, 2021
Merged

Use standard stream for loggers#1601
vfdev-5 merged 12 commits intopytorch:masterfrom
sdesrozis:logger_stream

Conversation

@sdesrozis
Copy link
Copy Markdown
Contributor

@sdesrozis sdesrozis commented Feb 1, 2021

Description:

By default, loggers stream on sys.stderr. This PR allows to define the stream and the default stream is now sys.stdout. This is useful when ignite is used with workload manager like slurm.

EDIT : replace default sys.stderr by std.stdout does not work in tests (check_config idist). It could be an issue capsys / pytest

EDIT 2 : It seems that https://pypi.org/project/pytest-catchlog/ provides fixture that helps to catch stream from logs.

Check list:

  • New tests are added (if a new feature is added)
  • New doc strings: description and/or example code are in RST format
  • Documentation is updated (if required)

@sdesrozis sdesrozis requested a review from vfdev-5 February 1, 2021 17:05
Copy link
Copy Markdown
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @sdesrozis !

@vfdev-5
Copy link
Copy Markdown
Collaborator

vfdev-5 commented Feb 1, 2021

@sdesrozis please, add a mention here about added arg, like here : https://github.com/pytorch/ignite/pull/1600/files

@sdesrozis sdesrozis requested a review from vfdev-5 February 2, 2021 08:42
@vfdev-5 vfdev-5 merged commit 5ead5fa into pytorch:master Feb 2, 2021
Copy link
Copy Markdown
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

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

Thanks Sylvain!

@sdesrozis sdesrozis deleted the logger_stream branch February 2, 2021 08:46
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.

2 participants