Skip to content

Changed ProgressBar init's type annotation to accept None#2541

Merged
vfdev-5 merged 7 commits intopytorch:masterfrom
Davidportlouis:progressBarFix
Apr 12, 2022
Merged

Changed ProgressBar init's type annotation to accept None#2541
vfdev-5 merged 7 commits intopytorch:masterfrom
Davidportlouis:progressBarFix

Conversation

@Davidportlouis
Copy link
Copy Markdown

@Davidportlouis Davidportlouis commented Apr 6, 2022 •
edited
Loading

Fixes #2540

Description:

  • Added None to bar_format type annotation in initialization argument of ignite.contrib.handlers.tqdm_logger.ProgressBar

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)

@github-actions github-actions bot added the module: contrib Contrib module label Apr 6, 2022
vfdev-5
vfdev-5 approved these changes Apr 6, 2022
View reviewed changes
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 @Davidportlouis , lgtm. Let's see if CI is passing and then we can merge it

@vfdev-5
Copy link
Copy Markdown
Collaborator

vfdev-5 commented Apr 6, 2022

@Davidportlouis code formatting checks are not passing: https://github.com/pytorch/ignite/runs/5858540569?check_suite_focus=true#step:11:64

Please reformat the code according to https://github.com/pytorch/ignite/blob/master/CONTRIBUTING.md#formatting-code
Thanks

@Davidportlouis
Copy link
Copy Markdown
Author

Davidportlouis commented Apr 8, 2022

@Davidportlouis code formatting checks are not passing: https://github.com/pytorch/ignite/runs/5858540569?check_suite_focus=true#step:11:64

Please reformat the code according to https://github.com/pytorch/ignite/blob/master/CONTRIBUTING.md#formatting-code Thanks

Fixed it

@sdesrozis
Copy link
Copy Markdown
Contributor

sdesrozis commented Apr 8, 2022

@Davidportlouis Tests are still failing because of string mismatch. Could you have a look please ? It would be great to run the tests locally before committing.

@Davidportlouis
Copy link
Copy Markdown
Author

Davidportlouis commented Apr 9, 2022

@Davidportlouis Tests are still failing because of string mismatch. Could you have a look please ? It would be great to run the tests locally before committing.

Sure i'll check it and run tests locally

@Davidportlouis
Copy link
Copy Markdown
Author

Davidportlouis commented Apr 9, 2022

@Davidportlouis Tests are still failing because of string mismatch. Could you have a look please ? It would be great to run the tests locally before committing.

Sure i'll check it and run tests locally

Fixed the string formatting issue, some tests (resume based tests) in tests/ignite/engine/test_deterministic.py are failing locally due to OSError - Too many open files. Other than that all tests including tests for tqdm_logger.py are passing

@vfdev-5 vfdev-5 enabled auto-merge (squash) April 9, 2022 15:29
@Davidportlouis Davidportlouis requested a review from vfdev-5 April 11, 2022 14:27
vfdev-5
vfdev-5 approved these changes Apr 12, 2022
View reviewed changes
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 @Davidportlouis , LGTM !

@vfdev-5 vfdev-5 merged commit 4153506 into pytorch:master Apr 12, 2022
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

Assignees

No one assigned

Labels

module: contrib Contrib module

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bar format is str but should be Optional[str] in ignite.contrib.handlers.tqdm_logger.ProgressBar

3 participants

@Davidportlouis @vfdev-5 @sdesrozis