The Wayback Machine - https://web.archive.org/web/20210502215210/https://github.com/mwouts/jupytext/pull/724
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

Follow-up on the new pre-commit hook #724

Merged
merged 43 commits into from Feb 4, 2021
Merged

Follow-up on the new pre-commit hook #724

merged 43 commits into from Feb 4, 2021

Conversation

@mwouts
Copy link
Owner

@mwouts mwouts commented Jan 21, 2021

Will close #722

@codecov
Copy link

@codecov codecov bot commented Jan 21, 2021

Codecov Report

Merging #724 (e8977f2) into master (a2f1409) will decrease coverage by 0.01%.
The diff coverage is 99.01%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #724      +/-   ##
==========================================
- Coverage   99.12%   99.10%   -0.02%     
==========================================
  Files          97      104       +7     
  Lines        9591     9862     +271     
==========================================
+ Hits         9507     9774     +267     
- Misses         84       88       +4     
Impacted Files Coverage Δ
jupytext/jupytext.py 98.32% <ø> (-0.01%) ⬇️
jupytext/cli.py 96.19% <95.83%> (-0.27%) ⬇️
jupytext/compare.py 99.35% <100.00%> (+0.01%) ⬆️
jupytext/config.py 97.54% <100.00%> (+0.03%) ⬆️
jupytext/pairs.py 100.00% <100.00%> (ø)
tests/conftest.py 100.00% <100.00%> (ø)
tests/test_cli.py 100.00% <100.00%> (ø)
tests/test_cli_config.py 100.00% <100.00%> (ø)
tests/test_pre_commit_0_ipynb_to_py.py 100.00% <100.00%> (ø)
tests/test_pre_commit_1_sync.py 100.00% <100.00%> (ø)
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a2f1409...e8977f2. Read the comment docs.

@Skylion007
Copy link
Contributor

@Skylion007 Skylion007 commented Jan 21, 2021

@asottile was kind enough to give us some feedback to pre-commit hook when I added it to the list of known pre-commit hooks: pre-commit/pre-commit.com#451 (comment)

@mwouts mwouts force-pushed the pre_commit_follow_up branch 3 times, most recently from 3c369e0 to 2a61551 Jan 21, 2021
mwouts added 22 commits Jan 20, 2021
The example fails because Jupytext rewrites the ipynb file:
[jupytext] Updating 'test.ipynb' with this change:
E               @@ -5,10 +5,10 @@
E                   "execution_count": 1,
E                   "metadata": {
E                    "execution": {
E               -     "iopub.execute_input": "2021-01-26T22:24:31.954956Z",
E               -     "iopub.status.busy": "2021-01-26T22:24:31.949570Z",
E               -     "iopub.status.idle": "2021-01-26T22:24:31.979381Z",
E               -     "shell.execute_reply": "2021-01-26T22:24:31.978412Z"
E               +     "iopub.execute_input": "2021-01-26T22:24:34.617791Z",
E               +     "iopub.status.busy": "2021-01-26T22:24:34.614232Z",
E               +     "iopub.status.idle": "2021-01-26T22:24:34.619679Z",
E               +     "shell.execute_reply": "2021-01-26T22:24:34.619940Z"
E                    }
E                   },
E                   "outputs": [
@mwouts mwouts force-pushed the pre_commit_follow_up branch from 749bb82 to ef9dd99 Jan 31, 2021
@mwouts mwouts force-pushed the pre_commit_follow_up branch from 10be285 to 188a6d0 Jan 31, 2021

# finally we move the paired notebook to a subfolder
tmpdir.mkdir("subfolder")
os.rename("test.py", "subfolder/test.py")

This comment has been minimized.

@Skylion007

Skylion007 Feb 2, 2021
Contributor

NIT: shutil.move should be used instead of os.rename . This can fail if you keep your git repos on a different drive/partition/filesystem from your tmpdir. Additionally, in cases like this where the filename remains the same, you can just specify the folder.

This comment has been minimized.

@mwouts

mwouts Feb 2, 2021
Author Owner

Sure I'll do that, thanks @Skylion007

mwouts added 3 commits Feb 2, 2021
and show paths with shlex.quote
@mwouts
Copy link
Owner Author

@mwouts mwouts commented Feb 2, 2021

@Skylion007 , @JohnPaton this version seems to work pretty well. If that sounds ok for you I'll merge this and release Jupytext v1.10.0, the first official tag with the pre-commit hook!

jupytext/cli.py Outdated Show resolved Hide resolved
mwouts added 2 commits Feb 3, 2021
@mwouts mwouts merged commit 22d82a8 into master Feb 4, 2021
29 checks passed
29 checks passed
skip_duplicate
Details
skip_duplicate
Details
Publish to PyPi
Details
Publish to PyPi
Details
pre-commit
Details
pre-commit
Details
Analyse Analyse
Details
Analyse Analyse
Details
test-pip (3.6)
Details
test-pip (3.6)
Details
test-pip (3.7)
Details
test-pip (3.7)
Details
test-pip (3.8)
Details
test-pip (3.8)
Details
test-pip (3.9)
Details
test-pip (3.9)
Details
test-conda (ubuntu-latest, 3.7) test-conda (ubuntu-latest, 3.7)
Details
test-conda (ubuntu-latest, 3.7) test-conda (ubuntu-latest, 3.7)
Details
test-conda (macos-latest, 3.7) test-conda (macos-latest, 3.7)
Details
test-conda (macos-latest, 3.7) test-conda (macos-latest, 3.7)
Details
test-conda (windows-latest, 3.7) test-conda (windows-latest, 3.7)
Details
test-conda (windows-latest, 3.7) test-conda (windows-latest, 3.7)
Details
test-pip-no-myst (3.7)
Details
test-pip-no-myst (3.7)
Details
CodeQL No new or fixed alerts
Details
LGTM analysis: Python No new or fixed alerts
Details
codecov/patch 99.01% of diff hit (target 80.00%)
Details
codecov/project/source 97.80% (target 97.00%)
Details
codecov/project/tests 100.00% (target 100.00%)
Details
@mwouts mwouts deleted the pre_commit_follow_up branch Feb 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants