New progress API #19

pfitzseb · 2016-10-22T14:22:38Z

Corresponding Juniper: http://discuss.junolab.org/t/juniper-progress-meters/893/4

Related: JunoLab/atom-julia-client#271 and JunoLab/atom-ink#90; supersedes #4.

Preview:

Some details:

The progress bar stack can hold determinate as well as indeterminate progress bars, but only the former will be displayed.
The global progress bar is the bottom-most determinate progress bar. If there are no determinate progress bars in the stack, it will be an indeterminate progress bar. If the stack is empty, it will be empty as well.
Timing calculations are done in julia-client right now, but could conceivably be done in ink instead. Let me know if anyone has any opinions on that.
The lower level API (everything not @progress) should be sufficiently powerful for most applications, but is easily extensible if not.
ProgressBar.msg (and therefore msg!()) don't have any effect on what's displayed right now. Originally I wanted to display msg as a tooltip when hovering over a progress bar, but I'm not sure if that's a good solution.

cc @ChrisRackauckas, @MikeInnes: Let me know what you think!

pfitzseb · 2016-10-24T14:55:02Z

So, things have gotten a bit prettier. And in the example above there's both versions of the API on display.

(The inner progress bar doesn't show any ETA because no once cares about a remaining time of < 1 second, so none will be displayed in that case)

pfitzseb · 2016-10-24T15:07:06Z

Also, does the next release have to support 0.4? I can't figure out how to get the macros working there (and don't rellay want to spend any more time on it anyways...).

ChrisRackauckas · 2016-10-24T15:24:59Z

I know I will be making heavy use of this, but am not supporting v0.4 in any of my packages anymore, but that's just me.

pfitzseb · 2016-10-25T09:47:53Z

Right. We also have the latest released version of Juno.jl working on 0.4, so maybe that's enough.

Apart from that: What are your thoughts for fallbacks if the code isn't running in Juno? I guess we could redirect our methods to ProgressMeter.jl or create our own implementation of a progress meter in the console, but I'm not super interested in that...

see http://discuss.junolab.org/t/juniper-progress-meters/893 for reference

and relocate progress.jl

codecov-io · 2016-10-26T14:09:44Z

Current coverage is 14.95% (diff: 37.50%)

Merging #19 into master will decrease coverage by 0.43%

@@             master        #19   diff @@
==========================================
  Files             7          8     +1   
  Lines            91        107    +16   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits             14         16     +2   
- Misses           77         91    +14   
  Partials          0          0

Powered by Codecov. Last update 1e914ed...471032a

MikeInnes · 2016-10-27T13:13:06Z

src/frontend.jl

@@ -77,3 +54,5 @@ structure(s::Ptr) = s
 structure(s::String) = s
 # TODO: do this recursively
 structure(x::Array) = x
+
+include("progress.jl")


This is easier to follow if it's included directly from Juno.jl

MikeInnes · 2016-10-27T13:14:55Z

src/progress.jl

+
+type ProgressBar
+  leftText::String
+  rightText::String


Are there any benefits to storing these properties rather than just sending messages to the frontend? Not sure that it's likely to cause issues, but generally it's a good idea to store any state in one place only.

Not really -- in theory it would be enough to have the id/uuid stored. It's just more convenient because we then only have to handle one update case on the Atom side.
Also, I'm lazy and didn't want to have to write Atom.msg("progress!", "update", Dict(:id => p.id, :rightText => s)) or similar.

I'll make the switch to not having any state in the julia object, seems a lot better.

MikeInnes · 2016-10-27T13:15:28Z

src/progress.jl

+  id::String
+  progress::Float64
+  msg::String
+  determinate::Bool


Given that progress and determinate are exclusive anyway, could we use a special value like NaN for indeterminate bars?

Yep. I'd go with negative values though.

MikeInnes · 2016-10-27T13:16:29Z

src/progress.jl

+Create a new progress bar and register it with Juno, if possible.
+"""
+function ProgressBar(;name = "", msg = "")
+  id = randstring(10)


If we're going to do IDs like this, may as well use a UUID

Yeah, I wanted to do that but was to lazy to figure out how to get them (since they aren't exported from Base).

MikeInnes · 2016-11-02T13:41:54Z

I'm playing around with this and it's awesome. I also think it's basically good to go across the board, modulo any final tweaks you want to make.

One thing – it's working great for me via @progress, but if I do j = ProgressBar(); progress!(j, n) nothing shows up. Any ideas why that is?

MikeInnes · 2016-11-02T13:42:25Z

Oh wait, I remember now, we clear everything when the client is done.

pfitzseb · 2016-11-02T13:43:39Z

Yeah, that's probably it. I thought about this a bit and now kinda like your idea of wrapping everything in a try ... finally ... and only clearing the indeterminate progress bar we create when a job starts.

MikeInnes · 2016-11-02T13:51:14Z

Ok great, I've made that tweak. As much as anything, making it possible to try the API interactively is a big boon.

also, parametrize them on whether Juno is running so we can easily define fallbacks for the REPL (maybe use Blink to show progress bars?)

MikeInnes · 2016-11-02T14:55:26Z

src/progress.jl

@@ -16,9 +13,10 @@ end
 Create a new progress bar and register it with Juno, if possible.
 """
 function ProgressBar(;name = "", msg = "")
-  id = string(Base.Random.uuid1())
-  p = ProgressBar(name, "", id, 0.0, msg)
+  p = ProgressBar{Val{isactive()}}(string(Base.Random.uuid1()))


FWIW, you could use ProgressBar{true} directly here.

However, I'm not sure what the aim is with storing this in the type. isactive() could change in which case this will be incorrect. In future I'd also like to handle the case where the user disconnects and reconnects to the running Julia session (though we don't have to handle that immediately of course).

Heh, just wanted to abuse multiple dispatch for the fallback ;)
But yeah, I'll revert to if isactive() ... else ... on every call.

and check for `isactive()` on every update call.

and improve docs

so let's get rid of the `!`

(I think?)

pfitzseb · 2016-11-02T19:32:06Z

Alright, I've changed the API a bit and added a new convenience method.

Are there any major issues left in any part of this, or can we merge soon?

MikeInnes

I can't think of any other major issues, so merge at your leisure.

pfitzseb added 9 commits October 26, 2016 15:53

implement julia side of a better progress API

5250aa1

see http://discuss.junolab.org/t/juniper-progress-meters/893 for reference

whoops

0da0993

isactive guards

5f394d5

move progress to it's own file

17ead1d

fix macro

3f77dac

add test for at-progress "name" for ...

7a45c46

export both APIs

2a14786

fix tests and depwarn on 0.4

35c4805

and relocate progress.jl

fix messed up rebase

e4b634c

pfitzseb force-pushed the sp/moreprogress branch from c4dfbbc to e4b634c Compare October 26, 2016 13:59

MikeInnes reviewed Oct 27, 2016

View reviewed changes

pfitzseb added 2 commits October 27, 2016 15:43

uuid, don't use ProgressBar.determinate

144d6c5

-!

cea1085

make progress bars stateless here

3371e20

also, parametrize them on whether Juno is running so we can easily define fallbacks for the REPL (maybe use Blink to show progress bars?)

MikeInnes reviewed Nov 2, 2016

View reviewed changes

pfitzseb added 6 commits November 2, 2016 16:02

always close macro'd progress bars

d4b935d

and check for `isactive()` on every update call.

fix

471032a

add do-syntax

14c3efd

and improve docs

export progress

a368e93

those methods don't mutate anything

416092e

so let's get rid of the `!`

better english

d049286

(I think?)

MikeInnes approved these changes Nov 2, 2016

View reviewed changes

pfitzseb merged commit be01960 into master Nov 2, 2016

ChrisRackauckas mentioned this pull request Nov 2, 2016

Time estimate in the progressbar #3

Closed

c42f mentioned this pull request May 13, 2017

[WIP] Flexible and convenient logging JuliaLang/Juleps#30

Merged

pfitzseb deleted the sp/moreprogress branch January 29, 2018 16:56

KSepetanc mentioned this pull request Oct 8, 2019

[BUG] Internal Error: UndefVarError: st not defined #391

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New progress API #19

New progress API #19

pfitzseb commented Oct 22, 2016 •

edited

Loading

pfitzseb commented Oct 24, 2016

pfitzseb commented Oct 24, 2016

ChrisRackauckas commented Oct 24, 2016

pfitzseb commented Oct 25, 2016

codecov-io commented Oct 26, 2016 •

edited

Loading

MikeInnes Oct 27, 2016

MikeInnes Oct 27, 2016

pfitzseb Oct 27, 2016

pfitzseb Nov 2, 2016

MikeInnes Oct 27, 2016

pfitzseb Oct 27, 2016

MikeInnes Oct 27, 2016

MikeInnes Oct 27, 2016

pfitzseb Oct 27, 2016

MikeInnes commented Nov 2, 2016

MikeInnes commented Nov 2, 2016

pfitzseb commented Nov 2, 2016

MikeInnes commented Nov 2, 2016

MikeInnes Nov 2, 2016

pfitzseb Nov 2, 2016

pfitzseb commented Nov 2, 2016

MikeInnes left a comment

New progress API #19

New progress API #19

Conversation

pfitzseb commented Oct 22, 2016 • edited Loading

pfitzseb commented Oct 24, 2016

pfitzseb commented Oct 24, 2016

ChrisRackauckas commented Oct 24, 2016

pfitzseb commented Oct 25, 2016

codecov-io commented Oct 26, 2016 • edited Loading

Current coverage is 14.95% (diff: 37.50%)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikeInnes commented Nov 2, 2016

MikeInnes commented Nov 2, 2016

pfitzseb commented Nov 2, 2016

MikeInnes commented Nov 2, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pfitzseb commented Nov 2, 2016

MikeInnes left a comment

Choose a reason for hiding this comment

pfitzseb commented Oct 22, 2016 •

edited

Loading

codecov-io commented Oct 26, 2016 •

edited

Loading