Simplify Binder build configuration and notebook execution

[willingc]

@shaunagm This WIP PR updates the Binder build.

The todo.md file has some recommendations after I ran all the notebooks. If you want to walk through this later this week, I would be happy to do so.

[willingc]

Looks like the missing file is due to a data directory/submodule not being copied to this repo.

JupyterLab's demo repo for Binder has a nice layout for binder with separate directories for data files and notebooks. Also it uses pyinvoke which simplifies a local install.

[shaunagm]

The missing file issue (the usacturial.txt issue) was due to a problem with how our setup.py was configured. The .txt file should actually be part of our HARK package which these notebooks use. Unfortunately fixing this for one notebook involves changing the version of all our notebooks, which we'd strongly prefer not to do.

And yes, let's do a videocall later this week?

[willingc]

Sure. I'm not clear on why the notebooks would need to all change version unless there are major breaking changes in EconARK's API.

I could set up another branch on my repo that pulls the latest commit from master for EconARK so at least you could test it out if you want. Should only take 5 minutes.

[shaunagm]

We don't want them to change version - we'd like to reach a point with most of these notebooks where they're "done" and we don't ever touch them again. Changing versions of Econ-ARK and then having to update the notebooks so they don't break when Econ-ARK has a breaking change is exactly what we're trying to avoid. But newer notebooks will need newer versions of Econ-ARK. If all notebooks in binder must have the same version of Econ-ARK we are trapped having to occasionally update 30+ notebooks to use new versions.

[willingc]

@shaunagm This branch https://github.com/willingc/DemARK/tree/try-master-latest-sha uses the latest master on Binder and it looks like stuff is working.

Look at the binder folder requirements.txt and see how the version for binder is configured to pull from master. The notebooks with the missing file are working now.

As an FYI, you can have binder serve a different version of Econ-ARK than the repo itself. Binder will default to taking the requirements in the binder folder first and if not found use the requirements at the repo's root.

Here's the binder link for a build using the master of econ-ARK: https://mybinder.org/v2/gh/willingc/DemARK/try-master-latest-sha

[shaunagm]

Oh, neat. Could we do something like git+git://github.com/econ-ark/HARK.git@latest_stable_release so that, unless overwritten in the notebook itself, the notebooks would automatically get whatever branch we'd created under the name latest_stable_release? (We already create release branches, so we could just duplicate stables one to latest_stable_release.) That's probably safer than just pinning to master, which we introduce bugs into all the time.

[willingc]

Yep you can pin it to any SHA1, tag, or release.

[llorracc]

This is related to a pet idea of mine, discussed and articulated in this PR <econ-ark/HARK#275>: Briefly, we should figure out how to add, to our testing/CI regime, the *.py files of some subset of the notebooks in the DemARK/REMARK repos. If we had had that infrastructure in place, we would have caught a subtle bug introduced by a PR that nearly derailed the weeklong class I am teaching in Budapest this week. Nobody seems to know an easy way to do this, which is surprising to me. which is to choose

…

On Tue, Jul 23, 2019 at 9:28 PM Carol Willing ***@***.***> wrote: @shaunagm <https://github.com/shaunagm> This branch https://github.com/willingc/DemARK/tree/try-master-latest-sha uses the latest master on Binder and it looks like stuff is working. Look at the binder folder requirements.txt and see how the version for binder is configured to pull from master. The notebooks with the missing file are working now. As an FYI, you can have binder serve a different version of Econ-ARK than the repo itself. Binder will default to taking the requirements in the binder folder first and if not found use the requirements at the repo's root. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#50?email_source=notifications&email_token=AAKCK74Z3UUGEUGN6YBGMH3QA5LWDA5CNFSM4IGHZLA2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2UFYKY#issuecomment-514350123>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKCK7ZKBRVMY267XE5NMNDQA5LWDANCNFSM4IGHZLAQ> .

-- - Chris Carroll

[willingc]

@llorracc Shauna and I chatted this morning. There are 3 general ways to test notebooks: nbval, nbconvert, and papermill. Typically using pytest as the test runner. A good resource for all things notebooks and reproducible research: https://github.com/jupyter-guide/jupyter-guide and its related https://github.com/jupyter-guide/ten-rules-jupyter.

[shaunagm]

@llorracc can you get us an initial list of notebooks you want to run, that should pass (or fail!) quickly?

Also: see this open active discussion on testing notebooks.

[llorracc]

@willingc Thanks very much for your extensive improvements on a bunch of fronts.

I was about to merge but then realized that this discussion is independently valuable and would be less prominent after a merge.

Having reread the discussion in the related issue I think my preferences have clarified.

Steps:

1. Get to a point where we have verified that ALL DemARKs and REMARKs work with the current stable release.
   • We are pretty close to that point right now; the Alphacruncher guys have tested all DemARKs and REMARKs and I think we have addressed most or all of the problems
2. Set things up so that, when there is a new dev release, that triggers a process to run all of the DemARK *.py files and all the REMARK do_min.py files
   • If there are notebooks that break, we can either:
     1. Do a "quick fix" which is to pin the notebooks to the previous dev release and put the unpinned version on a "to-fix" branch; or
     2. Actually fix the problem(s) and post the corrected versions

For every notebook that breaks, we therefore have a means to unbreak it.

I much prefer this workflow to pinning everything to the current (working) version, precisely because I think that trying to run existing notebooks on a new candidate release is an excellent tool for bug-finding in the new release.

[willingc]

I was about to merge but then realized that this discussion is independently valuable and would be less prominent after a merge.

No rush to merge. 😄

Having reread the discussion in the related issue I think my preferences have clarified.

Steps:

1. Get to a point where we have verified that ALL DemARKs and REMARKs work with the current stable release.

Excellent plan!

• We are pretty close to that point right now; the Alphacruncher guys have tested all DemARKs and REMARKs and I think we have addressed most or all of the problems

Nice!

2. Set things up so that, when there is a new dev release, that triggers a process to run all of the DemARK *.py files and all the REMARK do_min.py files

I would recommend to test the notebooks on all pushes to master and run the matrix on all supported Python versions and nightly. This way there should be minimal breakage at release time. Azure Pipelines may be a good CI tool instead of or in addition to Travis or CircleCI since it supports Windows and is usually much faster.

• If there are notebooks that break, we can either:

  1. Do a "quick fix" which is to pin the notebooks to the previous dev release and put the unpinned version on a "to-fix" branch; or
  2. Actually fix the problem(s) and post the corrected versions

For every notebook that breaks, we therefore have a means to unbreak it.

I much prefer this workflow to pinning everything to the current (working) version, precisely because I think that trying to run existing notebooks on a new candidate release is an excellent tool for bug-finding in the new release.

Yep, getting tests written would simplify quite a bit since you should be able to have binder use the latest stable version which would be pulled into the container if it is unpinned.

[llorracc]

Seems like the right way to do this is to have some way of tagging the notebooks that should be automatically run, maybe by adding something to the metadata, and then the script should just run those. Either that or have in the DemARK (and REMARK) repos a /test-me folder or something like that with a list of the names of the .py files that should be run when there is a pull request.