Use status bar for retriever update

[ghoshbishakh]

Fixes #397
Use a progress bar while scripts are being downloaded for retriever update

[henrykironde]

@ghoshbishakh great work. just a few things to change.
I think we can add Downlaod progress text = "\rDownload Progress: [{0}]

[henrykironde]

In the doc string we can remove this Example: 0.5 for 50%
or we wan instead explain how this comes up

"""Shows progressbar given progress in float

accepts a a number between 0 and 1 and it's converted to a float.
a number under 0 represents a 'halt'.
a number at 1 or greater represents 100%
"""

[ghoshbishakh]

@henrykironde thank you for taking a look. 😄 I am updating my PR.

[ghoshbishakh]

@henrykironde I hope that is better

[henrykironde]

how about we change the logic and assume that given a number just convert to float, and this case we may not need to check if its int.
so before line 102, we can just say progress = float(progress)

[ghoshbishakh]

good call 😄 will update.

[henrykironde]

Modify this to change the length of the progress bar
barWidth = 20

if we go with bar_length or barLenth , people reading the code later may understand easily since the comment is talking about length

[ghoshbishakh]

ok sure but actually I am dynamically setting the length of the progressbar later according to the console width. So I think its better to comment that as default bar width?

[henrykironde]

ok that works

[henrykironde]

space after the first line

[ghoshbishakh]

sorry I did not get that

[ghoshbishakh]

ah in the docstring ok!

[ghoshbishakh]

Did I miss anything?

[henrykironde]

if we are creating status, then I guess we may include it in line text = "\rDownload Progress:....
or we can remove the creation of status

[ghoshbishakh]

@henrykironde cat you take a look at it now?

[henrykironde]

ok

[ethanwhite]

Thanks for this @ghoshbishakh. Overall it looks great.

I have a couple of thoughts/recommendations/questions:

1. I think we can remove the reporthook function entirely and then get rid of the associated optional argument in the call to urllib.urlretriever.

2. The docstring for update_progressbar could cleaned up a little (for part an explanation of
   part of the cleanup see number 4 below). E.g.,

   """Show progressbar
   
   Takes a number between 0 and 1 to indicate progress from 0 to 100%.
   
   """
   

3. Is the initial declaration of bar_length there for cases when determining the
   size of the terminal doesn't work properly? If so then it would be clearer to
   include this in the except. If not, then could you explain a little more about
   what it's accomplishing.

4. Since update_progressbar shouldn't be getting values < 0 or > 1 I think we can
   probably get rid of lines 105-112, and 116-117 unless I'm
   missing something. If you were thinking about checking for bad values we could use
   an assert statement and throw an exception if the value isn't between 0 and 1.

5. I don't understand why it's necessary to call update_progressbar at both the
   beginning and end of the main for loop. Could you explain a little be more about
   why we need both of them.

[ghoshbishakh]

@ethanwhite thanks for the suggestions 😄

1. I am removing the reporthook
2. Updated docstring
3. Yes it was there for cases where determination of console size fails. I have put it in the except.
4. I was thinking that later may be we could use statuses like halt for any exception or like that. But yes its better to keep it simple
5. Actually the 0% is not displayed if we don't call it first once. So I will call it outside the for loop first then so that it isn't repeated uselessly?

[ghoshbishakh]

@ethanwhite I have pushed with the required changes

[henrykironde]

great work @ghoshbishakh

[ghoshbishakh]

@henrykironde thanks! 😄
Can you guide me a bit here:
numfocus/gsoc#105