Implement type-based API for progress records

[tkf]

close #7

This PR implements add Progress type that holds the progress value (fraction), name, its ID, parent ID, and a flag indicating if the task is finished or not (done):

https://github.com/JunoLab/ProgressLogging.jl/blob/d1e1b0ebd9048227bf1dd76141b851c35cc1c8a4/src/ProgressLogging.jl#L65-L71

This API is based on @c42f's suggestion #7 (comment) and I added Base.print(::IO, ::Progress) method to it. This way, the progress information is somewhat readable even without proper progress monitor:

julia> @info Progress(uuid4(), 0.1)
[ Info: Progress: 10%

There is a bit of machinery in @logprogress to help producing log record that is compatible with both old (untyped) and new (typed) APIs.

Here is an example of nested progress bar handling implemented in TerminalLoggers.jl JuliaLogging/TerminalLoggers.jl#13

[tkf]

I'll add/fix tests once we agree on the API.

[tkf]

@c42f It'd be nice if you can review this, too.

@oxinabox Are you also interested in this?

[tkf]

@oxinabox Thanks for the review! What do you think about Progress struct? In particular, what do you think about fraction = NaN vs fraction = missing? #13 (comment)

[c42f]

This looks pretty great to me. Needs some tests I suppose.

[c42f]

The word "task" here could be confused with Task. Maybe we should call them "jobs" or some other less overloaded term?

[tkf]

I use words like "sub-tasks", "child tasks" and "parent task" just after this line. Do "sub-jobs", "child jobs" and "parent job" sound OK?

[c42f]

Clever! Would the plan be to remove this and just use bare Progress in the future?

[tkf]

I think @pfitzseb wants to keep the original open/untyped API. I'm OK for keeping this as long as this compatible layer becomes annoying hard to maintain. ProgressString is somewhat already "too clever" but maybe it's within an OK range.

[c42f]

I'm not sure I followed everything which was going on with interpolation and macro hygiene here!

[tkf]

I think I had a trouble when using a macro inside a macro. I tend to give up fighting hygiene and escape the whole block when it happens. I'll see if I can avoid this after writing tests.

[c42f]

If you can make it neater by all means that would be great.

But I agree that sometimes manual escaping is just easier 😞

[tkf]

So, I tried to avoid interpolation but it didn't work. Here is a MWE:

julia> macro f(x)
           esc(:($Base.@isdefined $x))
       end
@f (macro with 1 method)

julia> macro g(x)
           :(@isdefined $(esc(x)))
       end
@g (macro with 1 method)

julia> let a = 1
           @f a
       end
true

julia> let a = 1
           @g a
       end
ERROR: LoadError: MethodError: no method matching @isdefined(::LineNumberNode, ::Module, ::Expr)
Closest candidates are:
  @isdefined(::LineNumberNode, ::Module, ::Symbol) at reflection.jl:265
in expression starting at REPL[26]:2

We could use Expr(:isdefined, var) (or Expr(:islocal, var)) directly like Base is doing:
JuliaLang/julia@4800158#diff-121162110854f09c3be5b39209646009L353

But I prefer sticking with the high-level API @isdefined since Expr(:isdefined, :x) is listed only at lowered AST section: https://docs.julialang.org/en/latest/devdocs/ast/#Lowered-form-1

[c42f]

But I prefer sticking with the high-level API @isdefined

Yes the special forms without surface syntax are implementation details of Base so best to avoid them.

I expect the error here is because nested macros see the Expr(:esc, ...) from the parent macro and @isdefined can't deal with that. It's an unsolved problem (JuliaLang/julia#23221 and all that).

[oxinabox]

@oxinabox Thanks for the review! What do you think about Progress struct? In particular, what do you think about fraction = NaN vs fraction = missing? #13 (comment)

When I did similar to this before in other places I usaed NaN, but using missing sounds much better.

[tkf]

Actually I'm thinking to use nothing now #13 (comment)

[codecov-io]

Codecov Report

Merging #13 into master will decrease coverage by 14.75%.
The diff coverage is 56.94%.

@@             Coverage Diff             @@
##           master      #13       +/-   ##
===========================================
- Coverage   90.27%   75.52%   -14.76%     
===========================================
  Files           1        1               
  Lines          72      143       +71     
===========================================
+ Hits           65      108       +43     
- Misses          7       35       +28

Impacted Files	Coverage Δ
src/ProgressLogging.jl	75.52% <56.94%> (-14.76%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0eae8c6...0805fff. Read the comment docs.

[tkf]

I added/fixed the tests. I think this is good to go now if the code looks good.

[c42f]

Looks good to me!

[tkf]

@pfitzseb @c42f @oxinabox Thanks a lot for reviewing this!