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

Using a subclass of tqdm leads to improper nesting #509

Closed
lauhayden opened this issue Feb 2, 2018 · 3 comments · Fixed by #1075 or #1085
Closed

Using a subclass of tqdm leads to improper nesting #509

lauhayden opened this issue Feb 2, 2018 · 3 comments · Fixed by #1075 or #1085
Assignees

Comments

@lauhayden
Copy link

from tqdm import tqdm
from time import sleep

class tqdm2(tqdm):
    pass

for i in tqdm(range(10), desc='1'):
    for j in tqdm2(range(10), desc='2'):
        sleep(0.1)

Expected Behaviour

Nested progress bars as usual

Observed Behaviour

Broken nesting: the tqdm labled '2' seems to override the tqdm labled '1', such that '1' is not shown while '2' is running.

Versions

tqdm 4.19.5
python 3.6.3

@casperdcl casperdcl self-assigned this Feb 2, 2018
@brownan
Copy link

brownan commented Aug 5, 2018

Just ran into this, as subclassing the tqdm class seems like a great way to make custom status bars, by overwriting the format_meter() function and such.

It looks like there are several places in the code that aren't designed with subclassing in mind. The first is in __new__:

    def __new__(cls, *args, **kwargs):
        # Create a new instance
        instance = object.__new__(cls)
        # Add to the list of instances
        if "_instances" not in cls.__dict__:
            cls._instances = WeakSet()
        if "_lock" not in cls.__dict__:
            cls._lock = TqdmDefaultWriteLock()
        with cls._lock:
            cls._instances.add(instance)
        # Create the monitoring thread
        if cls.monitor_interval and (cls.monitor is None or not
                                     cls.monitor.report()):
            try:
                cls.monitor = TMonitor(cls, cls.monitor_interval)
            except Exception as e:  # pragma: nocover
                warn("tqdm:disabling monitor support"
                     " (monitor_interval = 0) due to:\n" + str(e),
                     TqdmMonitorWarning)
                cls.monitor_interval = 0
        # Return the instance
        return instance

The instance set and monitoring thread are set as attributes of the class. They're meant to be shared among all instances, but a subclass will have a different cls.__dict__. It therefore makes a separate instance set and spins up a separate monitoring thread. I'm not sure what other problems this will cause.

Later when the _get_free_pos() classmethod is called, it looks in the cls._instances set, which is not shared among subclasses. So both bars will get the same position 0 and they sort of overwrite each other.

You can sort of work around this by explicitly setting the position in the constructor, but there's still problems when the bars are closed, as the _decr_instances() classmethod also looks in the cls._instances set to reposition the remaining bars.

I'm not sure what the best solution is to this. Keeping the instance set and monitoring thread as module globals may be a better option than class-level variables.

@brownan
Copy link

brownan commented Aug 5, 2018

Super hacky workaround is to put this in your subclass so they share the _instances set, regardless of which class initially creates it:

    def __new__(cls, *args, **kwargs):
        try:
            cls._instances = tqdm.tqdm._instances
        except AttributeError:
            pass

        instance = super().__new__(cls, *args, **kwargs)

        tqdm.tqdm._instances = cls._instances
        return instance

Each class still gets its own monitoring thread, but I don't think that hurts anything. All instances should also share a lock, but they seem to do that already because the TqdmDefaultWriteLock class is backed by module-level locks.

@casperdcl
Copy link
Sponsor Member

Ah I see so the issue is combining various different (sub)classes of tqdm which have the same output streams. Hmm. @brownan's patch is a pretty good work-around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants