-
Notifications
You must be signed in to change notification settings - Fork 7
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
Expose code blocks in replication and explain REPL usage #395
Conversation
Benchmark resultJudge resultBenchmark Report for /home/runner/work/Rt-without-renewal/Rt-without-renewalJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/Rt-without-renewal/Rt-without-renewalJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/Rt-without-renewal/Rt-without-renewalJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General comment is I am curious how I would run this vignette outside the root of the repo, as I had tried in my sandbox yesterday after installing the package. For me, this is typically how I gain familiarity with a new package, I will try to work with it in a separate scratch repo somewhere. But if the idea is more to work within this repo and load EpiAware, as long as it is clear to users how they could do this from their own project repo I think that's fine!
These are really good points. So eventually |
This should work too. You'll need add any other dependencies, but it should work without pinning to specific version. If you could test that out, that would be great. |
Because once it's on Julia registry, does the environment management that you have set up here:
Set this up automatically? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice changes but overall I think we want to aim for plug and play (i.e copy and paste interaction) and not requiring a local clone or any mucking about with our environments or local package.
I think either:
- People use it as a pluto notebook and we have instructions for them
- People copy and paste into their local instance.
I think those instructions should be in a central doc and we should aim to have as little boiler plate as possible in each doc.
Responding to the rest of the convo here all the current instructions etc just seem overly complicated and hacky. All we need is to show people what the dependencies are of a given tutorial in a block and let them install them via copying and pasting imo. |
Agreed. I'd independently come to that view as well and just pushed this change. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #395 +/- ##
=======================================
Coverage 93.33% 93.33%
=======================================
Files 53 53
Lines 540 540
=======================================
Hits 504 504
Misses 36 36 ☔ View full report in Codecov by Sentry. |
What I've done is to put the information about Pluto code blocks into FAQ, and the extra details about what env we want to use and checking out the branch version of EpiAware into developer docs. I think this should be usable/readable now. |
Benchmark resultJudge resultBenchmark Report for /home/runner/work/Rt-without-renewal/Rt-without-renewalJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/Rt-without-renewal/Rt-without-renewalJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/Rt-without-renewal/Rt-without-renewalJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
|
Benchmark resultJudge resultBenchmark Report for /home/runner/work/Rt-without-renewal/Rt-without-renewalJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/Rt-without-renewal/Rt-without-renewalJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/Rt-without-renewal/Rt-without-renewalJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some good updates. I think we are getting there. A few more comments for the developer docs and the showcase structure
Replying to a comment above:
I haven't done this... but an option is to stop using a development approach to The main downside is that a change to |
Benchmark resultJudge resultBenchmark Report for /home/runner/work/Rt-without-renewal/Rt-without-renewalJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/Rt-without-renewal/Rt-without-renewalJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/Rt-without-renewal/Rt-without-renewalJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
|
I think we should think about this in a new issue. There are upsides and downsides as you note and I'm not clear what the best choice is or if we can mitigate the downsides of either approach in some way to make the answer clear |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking really nice now. @kaitejohnson is you can have another go that would be amazing and report back on how you go.
IMO these things always benefit from a few rounds of user feedback to get a good pattern set up that most people can get the hang of quickly.
This PR expands the replication of Mishra 2022 in two ways:
RtWithoutRenewal
.I think this addresses the problem of this replication notebook requiring far too much from a first time user (e.g. close familiarity with
Pluto
, a desire to run the notebook locally in a way not described here) whilst not ditching the Pluto approach entirely. I've tried local build and running from the REPL... but I'm maybe not the best person to judge user friendliness.Very interested in views from @kaitejohnson.
Closes #319