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.

Sign up for GitHub

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

Jump to bottom

TrainsLogger -> ClearMLLogger #1557

Merged
merged 4 commits into from Jan 18, 2021
Merged

TrainsLogger -> ClearMLLogger #1557

merged 4 commits into from Jan 18, 2021

Conversation

ydcjeff
Copy link
Contributor

@ydcjeff ydcjeff commented Jan 16, 2021 •
edited

Fixes #1553

Description: Renamed TrainsLogger to ClearMLLogger in all files found. Let me know if I miss something.

Check list:

ydcjeff
ydcjeff commented Jan 16, 2021
View reviewed changes
ignite/contrib/engines/common.py Show resolved Hide resolved
ignite/contrib/handlers/clearml_logger.py Show resolved Hide resolved
ignite/contrib/handlers/trains_logger.py Show resolved Hide resolved
mypy.ini Show resolved Hide resolved
tests/ignite/contrib/engines/test_common.py Show resolved Hide resolved
tests/ignite/contrib/handlers/test_clearml_logger.py Show resolved Hide resolved
@sdesrozis
Copy link
Contributor

@ydcjeff thank you very much for this huge work. Although I need some time to review it... I do it as soon as possible !!

@ydcjeff
Copy link
Contributor Author

ydcjeff commented Jan 16, 2021

@ydcjeff thank you very much for this huge work. Although I need some time to review it... I do it as soon as possible !!

You can take time to review it. Since most of the changes are changing trains to clearml, I don't split into small PRs.

vfdev-5
vfdev-5 reviewed Jan 16, 2021
View reviewed changes
Copy link
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 a lot for the PR !
I checked it and it looks good. I added the answer to your question.

tests/ignite/contrib/handlers/test_clearml_logger.py Show resolved Hide resolved
@ydcjeff ydcjeff requested a review from vfdev-5 January 17, 2021 04:47
sdesrozis
sdesrozis reviewed Jan 17, 2021
View reviewed changes
Copy link
Contributor

@sdesrozis sdesrozis left a comment

Choose a reason for hiding this comment

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

It looks good for me ! Just let me until tomorrow to approve. Very well done !

sdesrozis
sdesrozis approved these changes Jan 18, 2021
View reviewed changes
Copy link
Contributor

@sdesrozis sdesrozis 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 !! Thank you !

vfdev-5
vfdev-5 approved these changes Jan 18, 2021
View reviewed changes
Copy link
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 @ydcjeff !

@vfdev-5 vfdev-5 merged commit 2485fd4 into pytorch:master Jan 18, 2021
@ydcjeff ydcjeff deleted the handlers/clearml branch January 18, 2021 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vfdev-5 vfdev-5 vfdev-5 approved these changes

@sdesrozis sdesrozis sdesrozis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rename Trains to ClearML
3 participants
@ydcjeff @sdesrozis @vfdev-5