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.

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

Fix file default value (ease redirection) #293

Merged
merged 4 commits into from May 29, 2017
Merged

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 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.

None yet

3 participants