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

Re-run notebooks that don't need human attention #266

Merged
merged 10 commits into from
Jan 10, 2022

Conversation

michaelosthege
Copy link
Member

I'm using the following script, located in pymc-examples/scripts and accompanied by todo.txt, error.txt and success.txt.
The todo.txt has a line with the absolute path to each notebook for which there is currently no other open pull request.

import pathlib
import rerun
import logging


logging.basicConfig(level=logging.INFO)
_log = logging.getLogger(__file__)


DP_SCRIPTS = pathlib.Path(__file__).absolute().parent
DP_REPO = DP_SCRIPTS.parent
FP_TODO = DP_SCRIPTS / "todo.txt"
FP_SUCCESS = DP_SCRIPTS / "success.txt"
FP_ERROR = DP_SCRIPTS / "error.txt"


def run(fp: pathlib.Path) -> bool:
    success = True
    success = success and rerun.apply_replacements(fp)
    success = success and rerun.run_precommit(fp)
    success = success and rerun.execute_notebook(fp)
    success = success and rerun.run_precommit(fp)
    success = success and rerun.commit(fp, "rerun-quickies")
    success = success and rerun.push("rerun-quickies", "mine")

    if success:
        _log.info("✔ All steps succeeded.")
        return True
    else:
        _log.error("❌ Manual investigation needed.")
        return False


if __name__ == "__main__":
    for fp in FP_TODO.open("r").read().split("\n"):
        already = {
            *FP_SUCCESS.open("r").read().split("\n"),
            *FP_ERROR.open("r").read().split("\n"),
        }
        if fp in already:
            _log.info("Skipping %s because it was already processed.", fp)
            continue

        if run(pathlib.Path(fp)):
            FP_SUCCESS.open("a").write(str(fp) + "\n")
        else:
            FP_ERROR.open("a").write(str(fp) + "\n")

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@michaelosthege michaelosthege changed the title ⏳ Re-run notebooks that didn't need human attention ⏳ Re-run notebooks that don't need human attention Jan 9, 2022
@review-notebook-app
Copy link

View / edit / reply to this conversation on ReviewNB

twiecki commented on 2022-01-09T15:39:11Z
----------------------------------------------------------------

We need to change one of the scripts I think so that the pymc3 tags do not get added, I did that with a replace also inside of .py files.


@review-notebook-app
Copy link

View / edit / reply to this conversation on ReviewNB

twiecki commented on 2022-01-09T15:39:12Z
----------------------------------------------------------------

The deep learning link won't work.


@review-notebook-app
Copy link

review-notebook-app bot commented Jan 9, 2022

View / edit / reply to this conversation on ReviewNB

twiecki commented on 2022-01-09T15:39:13Z
----------------------------------------------------------------

Also has deeplearning.net links.


michaelosthege commented on 2022-01-09T15:57:31Z
----------------------------------------------------------------

True, but out of scope. Broken Hyperlinks on Website · Issue #165 · pymc-devs/pymc-examples (github.com)

@review-notebook-app
Copy link

review-notebook-app bot commented Jan 9, 2022

View / edit / reply to this conversation on ReviewNB

twiecki commented on 2022-01-09T15:39:13Z
----------------------------------------------------------------

That didn't work.


michaelosthege commented on 2022-01-09T15:54:30Z
----------------------------------------------------------------

I have no idea why it was committed then. It must have had a 0 exit code.

@review-notebook-app
Copy link

review-notebook-app bot commented Jan 9, 2022

View / edit / reply to this conversation on ReviewNB

twiecki commented on 2022-01-09T15:39:15Z
----------------------------------------------------------------

Maybe use ConstantData.


michaelosthege commented on 2022-01-09T16:01:11Z
----------------------------------------------------------------

ConstantData was not part of 4.0.0b1 yet.

Expecting that we'll have to do more automated re-runs in the near future, I'd like to prioritize determining which notebooks (don't) need manual inspection..

@review-notebook-app
Copy link

View / edit / reply to this conversation on ReviewNB

twiecki commented on 2022-01-09T15:39:17Z
----------------------------------------------------------------

pm.ConstantData()


@review-notebook-app
Copy link

View / edit / reply to this conversation on ReviewNB

twiecki commented on 2022-01-09T15:39:18Z
----------------------------------------------------------------

pm.ConstantData


@review-notebook-app
Copy link

View / edit / reply to this conversation on ReviewNB

twiecki commented on 2022-01-09T15:39:18Z
----------------------------------------------------------------

pm.ConstantData()


Copy link
Member Author

I have no idea why it was committed then. It must have had a 0 exit code.


View entire conversation on ReviewNB

Copy link
Member Author

Copy link
Member Author

ConstantData was not part of 4.0.0b1 yet.

Expecting that we'll have to do more automated re-runs in the near future, I'd like to prioritize determining which notebooks (don't) need manual inspection..


View entire conversation on ReviewNB

@michaelosthege
Copy link
Member Author

Ready for review.
All other notebooks resulted in errors of varying severity.

@michaelosthege michaelosthege changed the title ⏳ Re-run notebooks that don't need human attention Re-run notebooks that don't need human attention Jan 9, 2022
@OriolAbril
Copy link
Member

Thanks, this is even worse than I expected 😅 and I thought that getting a 50% auto updated would be a huge success. We landed at 10/95

Related to #56 #217 #214 #80 #113 #90 #120 #121 #268

@OriolAbril
Copy link
Member

will merge in a few minutes after merging some other PRs and releasing a snapshot

@michaelosthege
Copy link
Member Author

michaelosthege commented Jan 9, 2022

Many also failed because I didn't have Bambi installed.
Also there might be a few more OK ones with the fixed pre-commit now. (I didn't restart from the beginning.)

Let's make a list of what we need to do manually - some of that might be candidates for the migration script, or for reversing unnecessary breaks at the API level.

@OriolAbril OriolAbril merged commit d3f8f0c into pymc-devs:main Jan 10, 2022
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.

2 participants