Skip to content
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

Add rerun script #264

Merged
merged 2 commits into from
Jan 9, 2022
Merged

Add rerun script #264

merged 2 commits into from
Jan 9, 2022

Conversation

michaelosthege
Copy link
Member

This PR adds scripts/rerun.py and applies it to the BEST notebook.
It can be used from the command line or from another Python script to apply API migrations, re-run notebooks and optionally commit to a specific branch and push it to a remote.

The replacements are the ones @twiecki wrote for #262.

For example, this is how we can apply it for the example scripts:

import rerun

for fp in ["example1.py", "example2.py"]:
    assert rerun.apply_replacements("some_example.py")
    assert rerun.commit(fp, "update-some-example")
assert rerun.push("update-some-example", "myremotename")

@twiecki @OriolAbril I think we should chunk the work and, for example, have one PR that updates only the example scripts like shown above.
If needed those PRs can then adapt the REPLACEMENTS.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

_log = logging.getLogger(__file__)
DP_REPO = pathlib.Path(__file__).absolute().parent.parent

REPLACEMENTS = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The order matters here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, so that's why there's the stundenatstudentt!
I though you were correcting typos there..

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we not make this an OrderedDict or can we rely on dict being ordered now?

@OriolAbril
Copy link
Member

This is related to #52, so we keep track of everything. I will also create a v4 (auto) column in the notebook tracker to take into account that they have been updated but might still need code updated to show v4 best practices, loops of scalar distributions converted to correct use of the size argument, removing now unnnecessary model factories things like that.

The PR LGTM, and merging this and aiming for multiple PRs also allows anyone to help.

@OriolAbril
Copy link
Member

#265 updates the BEST example on both code and style, can we autoupdate another notebook here to avoid merge issues?

@twiecki
Copy link
Member

twiecki commented Jan 9, 2022

What happened to the AB testing NB?

@michaelosthege
Copy link
Member Author

What happened to the AB testing NB?

I undid that commit to avoid conflicts, as requested by
#264 (comment)

The notebook I re-ran now needed a manual intervention, but I got annoyed by automated re-runs not working, so I just fixed it.
Most breaking changes that need manual intervention happen around working with InferenceData instead of MultiTrace.

@twiecki twiecki merged commit 043673b into pymc-devs:main Jan 9, 2022
@michaelosthege michaelosthege deleted the add-rerun-script branch January 9, 2022 11:29
@OriolAbril
Copy link
Member

related to #110

"arviz.from_pymc3": "pm.to_inference_data",
"pymc3": "pymc",
"PyMC3": "PyMC",
"pymc3": "pymc",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these occur twice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants