Skip to content

added logging sub-module#1155

Merged
casperdcl merged 2 commits intotqdm:develfrom
de-code:logging-redirect-to-tqdm
Apr 5, 2021
Merged

added logging sub-module#1155
casperdcl merged 2 commits intotqdm:develfrom
de-code:logging-redirect-to-tqdm

Conversation

@de-code
Copy link
Copy Markdown
Contributor

@de-code de-code commented Apr 1, 2021 •
edited
Loading

this implements a slightly improved version of #786

The solution mainly differs from the one outlined in #786 in the following ways:

  • multiple loggers can be specified (still optional, and probably not needed most of the time)
  • a tqdm class can be passed in
  • Python 2 support

@de-code de-code requested a review from casperdcl as a code owner April 1, 2021 19:35
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 1, 2021 •
edited
Loading

Codecov Report

Merging #1155 (7458666) into devel (9fb0f23) will increase coverage by 0.48%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##            devel    #1155      +/-   ##
==========================================
+ Coverage   89.32%   89.80%   +0.48%     
==========================================
  Files          25       26       +1     
  Lines        1658     1707      +49     
  Branches      275      283       +8     
==========================================
+ Hits         1481     1533      +52     
+ Misses        131      128       -3     
  Partials       46       46

de-code
de-code commented Apr 1, 2021
View reviewed changes
Comment thread tqdm/cli.py Outdated
@de-code de-code mentioned this pull request Apr 1, 2021
Closed
@de-code de-code force-pushed the logging-redirect-to-tqdm branch 3 times, most recently from 72b8be1 to c24dab3 Compare April 2, 2021 09:54
casperdcl
casperdcl requested changes Apr 3, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@casperdcl casperdcl left a comment

Choose a reason for hiding this comment

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

this is great. Perhaps should be in tqdm.contrib.logging or even just tqdm.contrib.__init__ for now?

https://github.com/tqdm/tqdm/#redirecting-writing still uses tqdm.contrib for the stdout/err redirection.

@casperdcl casperdcl added p3-enhancement 🔥 Much new such feature question/docs ‽ Documentation clarification candidate submodule ⊂ Periphery/subclasses labels Apr 3, 2021
@casperdcl casperdcl added this to the Non-breaking milestone Apr 3, 2021
@de-code
Copy link
Copy Markdown
Contributor Author

de-code commented Apr 3, 2021

this is great. Perhaps should be in tqdm.contrib.logging or even just tqdm.contrib.__init__ for now?

https://github.com/tqdm/tqdm/#redirecting-writing still uses tqdm.contrib for the stdout/err redirection.

I moved it to tqdm.contrib.logging for now. It might be good to have a separate module just for the logging related functions.
I also moved the tests accordingly (there didn't seem to be any tests for contrib yet).
I also added a section to the README.

@de-code de-code force-pushed the logging-redirect-to-tqdm branch from 7742d89 to 4125dcc Compare April 3, 2021 20:19
@de-code @casperdcl
added logging sub-module
e86b9b1 
added logging sub-module

python 2 compatibility

fixed python 2 fix

added test for custom tqdm class

python 2 absolute imports

(due to otherwise conflicting `logging` module)

isort

more tests relating to _get_first_found_console_formatter

isort

minor simplification

test handleError

test logging formatter being used

minor rename to _get_first_found_console_logging_formatter

test that certain exceptions are not swallowed

avoid using mock.assert_called (py 3.5)

moved to tqdm.contrib.logging

added "Redirecting console logging to tqdm" readme

removed no longer necessary absolute_import declaration

minor: updated package of example in docstring
@casperdcl casperdcl changed the base branch from master to devel April 5, 2021 22:46
@casperdcl casperdcl force-pushed the logging-redirect-to-tqdm branch from 4125dcc to 7458666 Compare April 5, 2021 23:52
@casperdcl
Copy link
Copy Markdown
Member

casperdcl commented Apr 5, 2021

fixes #786

@casperdcl casperdcl merged commit 3eeb09a into tqdm:devel Apr 5, 2021
@casperdcl casperdcl mentioned this pull request Apr 5, 2021
Merged
@de-code de-code deleted the logging-redirect-to-tqdm branch April 6, 2021 09:51
@casperdcl
Copy link
Copy Markdown
Member

casperdcl commented May 25, 2021

Hah @cthoyt it's a well overdue implementation of #786 - just never had enough free time to action it sooner; sorry. Full credit to @de-code though for one of the cleanest code PRs I've seen on this site.

user additions deletions
@de-code +429 -0
@casperdcl +102 -170

It may look like I modded a lot but most of that was documentation, not code.

@casperdcl casperdcl mentioned this pull request May 28, 2021
Open
@casperdcl casperdcl linked an issue May 28, 2021 that may be closed by this pull request
Pass progress bar to logger #313
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@casperdcl casperdcl casperdcl requested changes

Assignees

@de-code de-code

@casperdcl casperdcl

Labels

p3-enhancement 🔥 Much new such feature question/docs ‽ Documentation clarification candidate submodule ⊂ Periphery/subclasses

Projects

None yet

Milestone

Non-breaking

Development

Successfully merging this pull request may close these issues.

Pass progress bar to logger

3 participants

@de-code @casperdcl @cthoyt