shashi / FileTrees.jl

Parallel computing with a tree of files metaphor
http://shashi.biz/FileTrees.jl
Other
88 stars 6 forks source link

Add option to supply Context to Dagger #33

Closed DrChainsaw closed 3 years ago

DrChainsaw commented 3 years ago

Fixes #32

I don’t love the mocking-ish testcase, but I couldn’t think of another way to test that it is propagated correctly.

I chose to only make the exported compute functions (exec and compute) have non-context versions. Let me know if you prefer it for any other method as well.

I will run some more tests for my use case just to make sure and I’ll post right back here once I have done so.

codecov-commenter commented 3 years ago

Codecov Report

Merging #33 into master will increase coverage by 0.04%. The diff coverage is 87.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #33      +/-   ##
==========================================
+ Coverage   84.17%   84.22%   +0.04%     
==========================================
  Files           6        6              
  Lines         316      317       +1     
==========================================
+ Hits          266      267       +1     
  Misses         50       50              
Impacted Files Coverage Δ
src/parallelism.jl 80.76% <87.50%> (+0.76%) :arrow_up:

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 3a26a5c...74baeae. Read the comment docs.

shashi commented 3 years ago

Looks good to me!

DrChainsaw commented 3 years ago

Use case seems to work out at my end as well. Go ahead and merge when ready.

DrChainsaw commented 3 years ago

Ugh, just found an issue with mapexec. Just a sec and I'll patch it

DrChainsaw commented 3 years ago

I think I'm good now :)

Last commit might be poking a little bit too much into Dagger internals just to test that a context is never accidentally created with Context(). Feel free to exclude it or suggest a better way to test that a fallback never happened.

DrChainsaw commented 3 years ago

Travis build failed due to what seems like some random download error when fetching the registry. It seems like I can't restart it though

shashi commented 3 years ago

Looks great! :rocket: Thanks a lot!