Adding a script to standardize the notebook versions. #1431
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1431 +/- ##
=======================================
Coverage 100.0% 100.0%
=======================================
Files 218 218
Lines 14433 14433
=======================================
Hits 14426 14426
Misses 7 7 Continue to review full report at Codecov.
|
freddyaboulton
requested review from
angela97lin,
dsherry,
jeremyliweishih,
bchen1116,
christopherbunn and
eccabay
| click.echo(f"Set the notebook version to {desired_version} for:\n {different_versions}") | ||
|
|
||
|
|
||
| if __name__ == '__main__': |
There was a problem hiding this comment.
Ooo is this necessary because we're calling python docs/notebook_version_standardizer.py check-versions? Jw cause I know we had something similar for our CLI commands and then removed them :d
There was a problem hiding this comment.
Ah yeah, @angela97lin I think we had to remove that for codecov, right, before we loosened the coverage thresholding? I'm blanking on whether there was another reason... fundamentally, if this works it works!
There was a problem hiding this comment.
I think we don't need it for the cli because the setup.py file sets the entry point as __main__.py:cli()? Without this line, the cli won't run if you do python notebook_version_sanitizer.py
|
|
||
| .PHONY: lint-fix | ||
| lint-fix: | ||
| autopep8 --in-place --recursive --max-line-length=100 --select="E225,E222,E303,E261,E241,E302,E203,E128,E231,E251,E271,E127,E126,E301,W291,W293,E226,E306,E221" evalml | ||
| isort --recursive evalml | ||
| python docs/notebook_version_standardizer.py standardize |
| click.echo(f"Set the notebook version to {desired_version} for:\n {different_versions}") | ||
|
|
||
|
|
||
| if __name__ == '__main__': |
There was a problem hiding this comment.
Ah yeah, @angela97lin I think we had to remove that for codecov, right, before we loosened the coverage thresholding? I'm blanking on whether there was another reason... fundamentally, if this works it works!
|
|
||
|
|
||
| @cli.command() | ||
| @click.option('--desired-version', default='3.7.4', help='python version that all notebooks should match') |
There was a problem hiding this comment.
Perhaps we should standardize to the latest 3.8? I guess it doesn't really matter lol, unless there's some ugly version-dependent docs bug which pops up haha. I remember we surveyed the group and most people were running different python versions locally so that doesn't help us decide here 😆
There was a problem hiding this comment.
I'm going for 3.8.2 since that was the version of 3.8 that was in the notebooks before this PR. The good thing is that we can pass in the desired version in the makefile in the future
| if different_versions: | ||
| different_versions = ['\t' + notebook for notebook in different_versions] | ||
| different_versions = "\n".join(different_versions) | ||
| raise SystemExit(f"The following notebooks don't match {desired_version}:\n {different_versions}") |
There was a problem hiding this comment.
Could you please add something about how to fix? Like "please run make lint-fix to fix". Because this will now show up in the lint job failures.
freddyaboulton
force-pushed
the
1419-sanitize-notebook-versions
branch
from
5e8f0d2 to
4cc3056
Compare
Pull Request Description
Fixes #1419
This is a test to show that circle ci will fail when the notebook versions don't match: https://app.circleci.com/pipelines/github/alteryx/evalml/7250/workflows/8b47906b-6b9f-4f95-b0ee-c2f78f2a6d9c/jobs/80895
This is what the output looks like when lint fails:
Then you can fix with lint-fix:
After creating the pull request: in order to pass the release_notes_updated check you will need to update the "Future Release" section of
docs/source/release_notes.rstto include this pull request by adding :pr:123.