give a try to notebook stripout

added Documentation label

marked this merge request as draft

added 1 commit

• 1edf88d5 - Remove force nbstripout

Compare with previous version

It implies a new workflow.

For instance, in 1-ProceduralProgramming/1-Variables.ipynb cells are executed (no modification) and in 1-ProceduralProgramming/2-Conditions-and-loops.ipynb some code is modified and its cell is executed.

$ git ci -am "pouetpouet"
nbstripout...............................................................Failed
- hook id: nbstripout
- files were modified by this hook

It fails but it removes cells execution from 1-ProceduralProgramming/1-Variables.ipynb and 1-ProceduralProgramming/2-Conditions-and-loops.ipynb.

$ git ci -am "pouetpouet"
nbstripout...............................................................Passed
[pouet 6fa9cab] pouetpouet
 2 files changed, 1 insertion(+), 2 deletions(-)
$ git log
commit 6fa9cab24d01156d11405ad5bc99fd4ab029aeaf (HEAD -> pouet)
Author: Vincent Rouvreau <vincent.rouvreau@inria.fr>
Date:   Fri Feb 16 17:37:05 2024 +0100

    pouetpouet

commit 8307f307ad229fc65ff55c94b44947d12a2c45d6
...
$ git diff 8307f307ad229fc65ff55c94b44947d12a2c45d6..6fa9cab24d01156d11405ad5bc99fd4ab029aeaf
diff --git a/1-ProceduralProgramming/2-Conditions-and-loops.ipynb b/1-ProceduralProgramming/2-Conditions-and-loops.ipynb
index a171519..0ddd56e 100644
--- a/1-ProceduralProgramming/2-Conditions-and-loops.ipynb
+++ b/1-ProceduralProgramming/2-Conditions-and-loops.ipynb
@@ -50,7 +50,7 @@
     "    int a = 2;\n",
     "    \n",
     "    if (a > 0)\n",
-    "        std::cout << a << \" is greater than 0\" << std::endl;\n",
+    "        std::cout << a << \" is greater than 0 and pouet\" << std::endl;\n",
     "\n",
     "    if (a < 0)\n",
     "        std::cout << \"This line won't be executed so nothing will be printed.\" << std::endl;    \n",

Seems to work fine, but need to launch it twice. Tell me @sgilles if this new workflow is ok for you

I have checked it...

At first sight I didn't see the point of changing something that worked just fine.

But I tried your option, and it's in fact even better: if the only change done is that cell were executed, there are no empty commit, which was not the case with our former manual script.

So I'm completely for the new pipeline; the only possible drawback I see is if the developer forgets to activate the environment (won't happen if it is installed in the base Anaconda environment as I did on my laptop, but I understand why some never put anything in this base environment). I therefore wonder if we shouldn't encourage installation through pip?

This is absolutely not a blocking point; for me can be merged in its current state!

No, there is no need to install nbstripout with pip, as pre-commit install will do the job. cf. this stackoverflow

the only possible drawback I see is if the developer forgets to activate the environment

The "good way" to do so is to check it with continuous integration. I merge this one and open a new issue about it.

Mmmh, you lost me...

How to you intend to check with CI a user didn't forget the activate its local environment??? (or maybe you meant in CI checking that the behaviour is the correct one if he forgot?)

At any rate not top priority; as I said for me it's mergeable

approved this merge request

marked this merge request as ready

resolved all threads

merged

mentioned in commit aea8453b

I have just understood what you probably meant... that is checking in CI there are no activated cells before accepting the merge?

mentioned in issue #114 (closed)

mentioned in merge request sed-saclay/cppyy_kernel!25 (merged)