mwouts / jupytext

Jupyter Notebooks as Markdown Documents, Julia, Python or R scripts
https://jupytext.readthedocs.io
MIT License
6.6k stars 386 forks source link

Don't build the extension in pre-commit environments #1226

Closed mwouts closed 4 months ago

mwouts commented 5 months ago

With this PR, the extension is not build when the Python executable looks like a pre-commit environment.

Closes #1210

After this PR, the test tests/external/pre_commit/test_pre_commit_1_sync_with_no_config.py runs in 25 seconds (was 54 seconds previously).

At least this should fix the issue that Jupytext can't be installed in a pre-commit hook env when npm is not present.

If I new how to do that, I would prefer to not even take a dependency on hatch-jupyter-builder and jupyterlab (I think that's where most of the 25 seconds come from).

I see that it is possible to have dynamical values in the project table (e.g. version, or even dependencies, so I have been thinking of writing a hook that would identify pre-commit environments and set e.g. a build_for_pre_commit_env variable, but then @mahendrapaipuri do you think I could use that variable to skip the extension build?

github-actions[bot] commented 5 months ago

Thank you for making this pull request.

Did you know? You can try it on Binder: Binder:lab or Binder:notebook.

Also, the version of Jupytext developed in this PR can be installed with pip:

pip install git+https://github.com/mwouts/jupytext.git@no_extension_in_pre_commit_envs

(this requires nodejs, see more at Developing Jupytext)

mwouts commented 5 months ago

Apparently I have been able to require jupyterlab>=4 dynamically, and now the test runs in only 8 seconds.

mwouts commented 5 months ago

Apparently I have been able to require jupyterlab>=4 dynamically, and now the test runs in only 8 seconds.

Well apparently that did not work :cry: (the dev install fails on the CI)

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 97.73%. Comparing base (642c709) to head (c607e7d).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1226 +/- ## ======================================= Coverage 97.73% 97.73% ======================================= Files 29 29 Lines 4463 4463 ======================================= Hits 4362 4362 Misses 101 101 ``` | [Flag](https://app.codecov.io/gh/mwouts/jupytext/pull/1226/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts) | Coverage Δ | | |---|---|---| | [external](https://app.codecov.io/gh/mwouts/jupytext/pull/1226/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts) | `75.14% <ø> (+0.11%)` | :arrow_up: | | [functional](https://app.codecov.io/gh/mwouts/jupytext/pull/1226/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts) | `88.48% <ø> (ø)` | | | [integration](https://app.codecov.io/gh/mwouts/jupytext/pull/1226/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts) | `77.28% <ø> (ø)` | | | [unit](https://app.codecov.io/gh/mwouts/jupytext/pull/1226/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts) | `66.55% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mwouts commented 4 months ago

As discussed above, I have merged #1233 instead - thanks @mahendrapaipuri for your advice!