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

New progress bar with estimated remaining time. #4

Merged
merged 22 commits into from Feb 19, 2016

Conversation

Projects
None yet
2 participants
@zzawadz
Contributor

zzawadz commented Feb 16, 2016

I love your package. It's very helpful, but I have always missed one functionality - printing of estimated remaining time.

So I have added new, custom progress bar to your package with this time estimation:

pbreplicate(1000, Sys.sleep(runif(1, 0.01)))
# |+++                                                                           | 2.70% ~09m 05s
pbreplicate(100000, Sys.sleep(runif(1, 0.1)))
# |+                                                                               | 0.02% ~13h 08m 28s   

I hope it will be usefull:)

pb = startpb(max = 100)
 # |                                                                                  | 0.00% ~~calculating  
  pb()
 # |+                                                                                | 1.00% ~00s
@psolymos

This comment has been minimized.

Show comment
Hide comment
@psolymos

psolymos Feb 16, 2016

Owner

Thanks @zzawadz ! It looks like a cool and useful feature. I'll review it shortly. I am not sure that I will change the default option though.

Owner

psolymos commented Feb 16, 2016

Thanks @zzawadz ! It looks like a cool and useful feature. I'll review it shortly. I am not sure that I will change the default option though.

psolymos added some commits Feb 17, 2016

@psolymos

This comment has been minimized.

Show comment
Hide comment
@psolymos

psolymos Feb 17, 2016

Owner

OK, so it works and does the job right, thanks. I called it 'timer' instead of 'custom' and is still the default because I think it is neat. I did some timings and couldn't detect any noticeable overhead due to extra calculations and string processing.

I'd like to follow the functions getTxtProgressBar, setTxtProgressBar, and txtProgressBar more closely, so that the timer functionality works outside of the pbapply functions. Something like adding getTimerProgressBar, setTimerProgressBar, and timerProgressBar.

Owner

psolymos commented Feb 17, 2016

OK, so it works and does the job right, thanks. I called it 'timer' instead of 'custom' and is still the default because I think it is neat. I did some timings and couldn't detect any noticeable overhead due to extra calculations and string processing.

I'd like to follow the functions getTxtProgressBar, setTxtProgressBar, and txtProgressBar more closely, so that the timer functionality works outside of the pbapply functions. Something like adding getTimerProgressBar, setTimerProgressBar, and timerProgressBar.

@zzawadz

This comment has been minimized.

Show comment
Hide comment
@zzawadz

zzawadz Feb 17, 2016

Contributor

Ok. I'll rewrite it for this progressBar scheme:)

Contributor

zzawadz commented Feb 17, 2016

Ok. I'll rewrite it for this progressBar scheme:)

@psolymos

This comment has been minimized.

Show comment
Hide comment
@psolymos

psolymos Feb 17, 2016

Owner

I already had a look and the best way is probably have elapsed time (since start time) stored with the value, so that the get* function can retrieve it, and just have it printed. I couldn't finish last night, but based on what you already had it seemed quite doable.

I was also thinking about replacing the progress bar (|+++++ |) with a throbber. For example the following characters would replace each other: |/-\-. This would indicate that calculations are ongoing, while the time estimate would give an idea of how long. Again, named as throbberProgressBar or something.

Owner

psolymos commented Feb 17, 2016

I already had a look and the best way is probably have elapsed time (since start time) stored with the value, so that the get* function can retrieve it, and just have it printed. I couldn't finish last night, but based on what you already had it seemed quite doable.

I was also thinking about replacing the progress bar (|+++++ |) with a throbber. For example the following characters would replace each other: |/-\-. This would indicate that calculations are ongoing, while the time estimate would give an idea of how long. Again, named as throbberProgressBar or something.

@zzawadz

This comment has been minimized.

Show comment
Hide comment
@zzawadz

zzawadz Feb 17, 2016

Contributor

Ok. But if you want, I can do this for you tomorrow:)

Maybe we should pack this progress bar in reference class, or sth similar to it?

Contributor

zzawadz commented Feb 17, 2016

Ok. But if you want, I can do this for you tomorrow:)

Maybe we should pack this progress bar in reference class, or sth similar to it?

@psolymos

This comment has been minimized.

Show comment
Hide comment
@psolymos

psolymos Feb 17, 2016

Owner

What do you mean by reference class?

I can wait a day or two so that you can write it up :) I'll have you then as package author if you are OK with that.

Owner

psolymos commented Feb 17, 2016

What do you mean by reference class?

I can wait a day or two so that you can write it up :) I'll have you then as package author if you are OK with that.

@zzawadz

This comment has been minimized.

Show comment
Hide comment
@zzawadz

zzawadz Feb 18, 2016

Contributor

I thought about R RefClass (http://www.inside-r.org/r-doc/methods/referenceclasses), but I looked at the implementation of txtProgressBar, and changed my mind.

Now there is a function called timerProgressBar - and it has similar interface as txtProgressBar. So that setTimerProgressBar, and getTimerProgresBar are just:

setTimerProgressBar <- setTxtProgressBar
getTimerProgressBar <- getTxtProgressBar

timerProgressBar inherits from txtProgressBar:)

I think it is more consistent.

Contributor

zzawadz commented Feb 18, 2016

I thought about R RefClass (http://www.inside-r.org/r-doc/methods/referenceclasses), but I looked at the implementation of txtProgressBar, and changed my mind.

Now there is a function called timerProgressBar - and it has similar interface as txtProgressBar. So that setTimerProgressBar, and getTimerProgresBar are just:

setTimerProgressBar <- setTxtProgressBar
getTimerProgressBar <- getTxtProgressBar

timerProgressBar inherits from txtProgressBar:)

I think it is more consistent.

@psolymos psolymos merged commit 24b6716 into psolymos:master Feb 19, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@psolymos

This comment has been minimized.

Show comment
Hide comment
@psolymos

psolymos Feb 19, 2016

Owner

Merged the PR, thanks for the nice contribution (check out authors field in DESCRIPTION).

Note:

  • I added couple of more arguments
  • defined .killed for closeing the connection
  • exported the new functions,
  • wrote Rd file.
Owner

psolymos commented Feb 19, 2016

Merged the PR, thanks for the nice contribution (check out authors field in DESCRIPTION).

Note:

  • I added couple of more arguments
  • defined .killed for closeing the connection
  • exported the new functions,
  • wrote Rd file.
@zzawadz

This comment has been minimized.

Show comment
Hide comment
@zzawadz

zzawadz Feb 19, 2016

Contributor

Thanks:)

Contributor

zzawadz commented Feb 19, 2016

Thanks:)

@psolymos

This comment has been minimized.

Show comment
Hide comment
@psolymos

psolymos Mar 1, 2016

Owner

Version 1.2-0 is on CRAN now and tagged: https://github.com/psolymos/pbapply/releases/tag/v1.2-0

Owner

psolymos commented Mar 1, 2016

Version 1.2-0 is on CRAN now and tagged: https://github.com/psolymos/pbapply/releases/tag/v1.2-0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment