This pull request fixes 10 alerts when merging 94732f4 into 62c5b45 - view on LGTM.com

fixed alerts:

• 9 for Mismatch between signature and use of an overridden method
• 1 for Multiple calls to __init__ during object initialization

Very nice work! I was actually working on something similar (and still working on writing a response to your elaborate comment) but this is excellent!

I was using a slightly slimpler method and I'm not sure about the upsides or downsides. I simply changed the __iter__ to be a generator and removed the __next__. I need to evaluate though, but I'm planning on creating a new release featuring this within about a week

Having __iter__ return a new object was my initial inclination also, but that means that when you're iterating there's two objects that share a job - the ProgressBar iterator and the generator.

It took me a while to work out if it was possible to combine them - it came down to needing to manually put in the __del__ method. With that, I think this promotingProgressBar to Generator is more elegant bc it saves us the extra object.

I can't think of any other pro/con. I think they're behaviorally the same other than that, although I'd be interested to hear if that's wrong - Python is always more weird than i think :p

I don't mean a new object, I meant replacing __iter__ with a generator.

Here's the current code: https://github.com/WoLpH/python-progressbar/blob/62c5b4543efd00b155898c2fe18fdbfa5a6b9e12/progressbar/bar.py#L522-L536

I was thinking of replacing it with something like this:

    def __iter__(self):
        try:
            if self.start_time is None:
                self.start()

            for value in self._iterable:
                self.update(self.value + 1)
                yield value
        except StopIteration:
            self.finish()
        except GeneratorExit:
            self.finish(dirty=True)

When __iter__ is a generator function, calling it returns a new generator object, which is the second object I was trying to describe above.

class IterReturnsGenerator:
    def __iter__(self):
        yield "I'm a Generator"
        return

irg_class_object = IterReturnsGenerator()
print(type(irg_class_object)) # <class '__main__.IterReturnsGenerator'>
print(type(irg_class_object.__iter__)) # <class 'method'>

irg_generator_object = irg_class_object.__iter__()

print(type(irg_generator_object)) # <class 'generator'>
print(id(irg_class_object) == id(irg_generator_object)) # False

When the class in question is a Generator itself, and __iter__ returns self, the last print would come out true, and there'd only be one object allocated in memory.

When __iter__ is a generator function, calling it returns a new generator object, which is the second object I was trying to describe above.

You're right, I hadn't tested it well and I was wondering why I chose for this method when writing it some years ago ;)

I've finally had some time to test this and it does look like it fixes a few issues but definitely not all of them. For example, try this test:

import sys
import time 
import progressbar 


try:
    bar = progressbar.ProgressBar(fd=sys.stdout, redirect_stdout=True)
    for i in bar(range(20)): 
        print('Some text', i) 
        time.sleep(0.2) 
        if i == 5:
            raise KeyboardInterrupt
except:
    pass

print('a')
time.sleep(0.3)
print('b')
time.sleep(0.3)
print('c')
time.sleep(0.3)
print('d')
time.sleep(0.3)
print('e')
time.sleep(0.3)
print('f')

That returns:

Some text 0
Some text 1
Some text 2
Some text 3
Some text 4
Some text 5
a
b
c
d
e
f
 25% (5 of 20) |############                                      | Elapsed Time: 0:00:02 ETA:   0:00:15%

Still though, as soon as I've got the tests fully working again I'll create a new release :)

Yeah, I can see the behavior in your test being kinda weird;
Our solution does at least close out the progress bar, so that's a plus.
But it does it at the end of the test when the ProgressBar object is garbage collected; until then the bar variable holds a reference to it, so it can't be collected. If the KeyboardInterrupt wasn't caught and ignored, it would have triggered the garbage collector and the bar would have been closed straight away, which would be the normal case for interrupts in real life.

Do you have an idea what behavior you'd like in the above case?

Apologies for the really slow response, it's been a very busy month and I do very much appreciate all the help :)

I'm still working on merging this. Specifically fixing the tests, for some reason the output redirection breaks as soon as the progressbar becomes a generator. Not entirely sure what the reason is but I'll get it working one way or another.

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.