Skip to content

Conversation

lrq3000
Copy link
Member

@lrq3000 lrq3000 commented Oct 17, 2016

Implements #275.

file argument is now set to None and defined at instanciation inside __init__() (instead of at class import). This should ease file redirection (as long as sys.stdout is redirected before tqdm instanciation, it should be fine).

TODO:

  • Use file=None and set default self.fp=sys.stderr inside init. Same for tqdm.write() (and other methods/submodules?). This will allow users to redirect sys.stderr before instanciating tqdm, without needing to supply the new redirection to tqdm.
  • Check that it doesn't impact the stdout redirection scheme described in the readme.
  • Add unit test for stdout (or a dummy IO) redirection (2 tests: 1- redirect using wrapper like in README, 2- redirect stdout before instanciating tqdm, this is the new redirection scheme provided by this PR).

TODO in a separate PR resiliency-io-error:

  • Wrap any self.fp.write() (tqdm.move() and status_printer?) in try/except block (this should not impact performances as the except block will only get evaluated if necessary). The except block should still display any error but as a non blocking message (using a new TqdmErrorMessage() class?). Something like this:
def move(n):
    try:
        #...
    except IOError as exc:
        if not self.resiliency:
            raise(exc)

@lrq3000 lrq3000 added the p3-enhancement 🔥 Much new such feature label Oct 17, 2016
@codecov-io
Copy link

codecov-io commented Oct 17, 2016

Codecov Report

❗ No coverage uploaded for pull request base (master@76fe211). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master     #293   +/-   ##
=========================================
  Coverage          ?   92.05%           
=========================================
  Files             ?        7           
  Lines             ?      579           
  Branches          ?      131           
=========================================
  Hits              ?      533           
  Misses            ?       44           
  Partials          ?        2

@lrq3000
Copy link
Member Author

lrq3000 commented Oct 17, 2016

I checked that redirection works correctly with this code (maybe we should add a unit test?):

from time import sleep

import contextlib
import sys

from tqdm import tqdm

class DummyTqdmFile(object):
    """Dummy file-like that will write to tqdm"""
    file = None
    def __init__(self, file):
        self.file = file

    def write(self, x):
        # Avoid print() second call (useless \n)
        if len(x.rstrip()) > 0:
            tqdm.write(x, file=self.file)

@contextlib.contextmanager
def stdout_redirect_to_tqdm():
    save_stdout = sys.stdout
    try:
        sys.stdout = DummyTqdmFile(sys.stdout)
        yield save_stdout
    # Relay exceptions
    except Exception as exc:
        raise exc
    # Always restore sys.stdout if necessary
    finally:
        sys.stdout = save_stdout

def blabla():
    print("Foo blabla")

# Redirect stdout to tqdm
with stdout_redirect_to_tqdm() as save_stdout:
        # tqdm call need to specify sys.stdout, not sys.stderr (default)
        # and dynamic_ncols=True to autodetect console width
        for _ in tqdm(range(3), file=save_stdout, dynamic_ncols=True):
            blabla()
            sleep(.5)

# After the `with`, printing is restored
print('Done!')

@casperdcl casperdcl force-pushed the master branch 4 times, most recently from 8cade97 to a65e347 Compare October 31, 2016 02:34
@casperdcl casperdcl added this to the v5.0.0 milestone Nov 13, 2016
@casperdcl casperdcl force-pushed the file-default-fix branch from 63433ca to 186a8dd Compare May 1, 2017 16:09
@casperdcl casperdcl added to-merge ↰ Imminent to-review 🔍 Awaiting final confirmation labels May 5, 2017
@casperdcl casperdcl self-assigned this May 5, 2017
@casperdcl casperdcl removed the to-review 🔍 Awaiting final confirmation label May 5, 2017
@casperdcl casperdcl merged commit 204fc43 into master May 29, 2017
@casperdcl casperdcl deleted the file-default-fix branch May 29, 2017 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p3-enhancement 🔥 Much new such feature to-merge ↰ Imminent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants