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

how to use nbdime to only commit code cell changes? #410

Closed
stas00 opened this issue Aug 4, 2018 · 13 comments
Closed

how to use nbdime to only commit code cell changes? #410

stas00 opened this issue Aug 4, 2018 · 13 comments

Comments

@stas00
Copy link

stas00 commented Aug 4, 2018

I have nbdime configured to be used as a driver for 'git diff' and 'git merge':

nbdime config-git --enable --global

and I have nbdime configured to ignore all but code cells: (~/.jupyter/nbdime_config.json):

{

  "Extension": {
    "source": true,
    "details": false, 
    "outputs": false,
    "metadata": false
  },

  "NbDiff": {
    "source": true,
    "details": false, 
    "outputs": false,
    "metadata": false
  },

  "NbDiffDriver": {
    "source": true,
    "details": false, 
    "outputs": false,
    "metadata": false
  },

  "NbMergeDriver": {
    "source": true,
    "details": false, 
    "outputs": false,
    "metadata": false
  },

  "dummy": {}


}

so now when I do 'git diff' I get:

## modified /cells/4/source:
@@ -1,3 +1,4 @@
+#test
 from pathlib import Path

which is perfect.

Now, the notebook under git control has everything committed (outputs, metadata, etc.). I want git to ignore any changes in outputs, metadata and details and commit only changes in code cells, exactly as it's shown in 'git diff' (via nbdime configured to only diff code cells).

How can I commit just the code changes?

I tested what I have now and I get all changes committed, not just code cells.

One way nbdime could do that is to:

  • extract code cells diff
  • replace modified notebook with the clean checked out version before any changes
  • apply the diff
  • commit

but I don't know how to make nbdime to do that (or perhaps it does it already).

and I realize that the diff might not apply, if there are cells that were added and removed, so perhaps this is not an option at all.

edit: So we ended up using nbstripout to remove all but what's needed to be stored under git. Next is to figure out how to do automated merging a stripped out notebook under git with the local unstripped out notebook. I was thinking that perhaps nbmerge could mirror nbstripout's logic and be able to automatically merge the two, knowing which cells are important and need to be merged and which cells can be ignored.

Thank you.

@vidartf
Copy link
Collaborator

vidartf commented Aug 13, 2018

Thanks for the update. Using a filter is indeed the right way to go. I think it will be hard to automatically resolve a merge between a version which is cleaned and one that is not. If you could break down the logic of how you imagine that working, maybe we can find a solution.

@stas00
Copy link
Author

stas00 commented Aug 14, 2018

I was talking about the case where a local notebook has changes in fields that aren't under git (i.e. those fields that get nbstripped out before commit) (e.g. execution_count). But I think before I can figure out the logic, I need to get out the basic conflicts out of the way.

Consider dev_nb/004_callbacks.ipynb modified locally and also it has been changed in HEAD since checkout - a perfect merge scenario:

$ git pull
Updating 6ab94c0..f02fb38
error: Your local changes to the following files would be overwritten by merge:
        dev_nb/004_callbacks.ipynb
Please commit your changes or stash them before you merge.
Aborting

$ git mergetool --tool nbdime -- dev_nb/004_callbacks.ipynb
No files need merging

$ git stash
Saved working directory and index state WIP on master: 6ab94c0 add some more instructions

$ git pull
Updating 6ab94c0..64f6d7d
error: Your local changes to the following files would be overwritten by merge:
        dev_nb/004_callbacks.ipynb
Please commit your changes or stash them before you merge.
Aborting

This shouldn't happen, since git stash should have removed all local changes. But it doesn't do a full stash because nbstripout gets in the way.

Once I disable nbstripout filter the above workflow does work.

I think what I'm trying to do is find a way around the conflict with filters and merging. Specifically if 'git pull' tells me:

error: Your local changes to the following files would be overwritten by merge:
        dev_nb/004_callbacks.ipynb
Please commit your changes or stash them before you merge.
Aborting

I'd like to run nbmerge to do the merge right after git pull that aborts, without needing to disable nbstripout.

Can you see such a way?

Thank you.

I found a document that also talks about how to overcome this conflict by constantly installing/uninstalling nbsrtripout https://lightlab.readthedocs.io/en/master/_static/misc/mergeWithNotebooks.html

@vidartf
Copy link
Collaborator

vidartf commented Aug 15, 2018

This is more of a git issue than an issue of nbdime, so you might have more luck trying in another forum (e.g. stackoverflow). That being said, I can think of two ways to deal with your stashing issue:

  1. Disable the filter during the stash command with -c filter.nbstripout.clean=cat option.
  2. OR, do a reset after your stash: git stash && git reset --hard HEAD.

@stas00
Copy link
Author

stas00 commented Aug 17, 2018

I posted on SO as you suggested.

Thank you for the suggestions, but neither of them works.

  1. git stash push has no -c option
  2. Running git reset --hard HEAD after git stash doesn't change anything. git pull still fails in the same way.

@vidartf
Copy link
Collaborator

vidartf commented Aug 20, 2018

BTW: What happens if you use the command git pull --rebase?

@vidartf
Copy link
Collaborator

vidartf commented Aug 20, 2018

Also, did you try asking on the nbstripout repo?

@stas00
Copy link
Author

stas00 commented Aug 23, 2018

BTW: What happens if you use the command git pull --rebase?

git pull --rebase origin master
error: cannot pull with rebase: You have unstaged changes.
error: please commit or stash them.

Also, did you try asking on the nbstripout repo?

Not yet, I'm going to spend a bit more time trying to debug it, and if all fails then will ask there next.

Thank you for your continuous support, Vidar.

@vidartf
Copy link
Collaborator

vidartf commented Aug 23, 2018

At least this should work, I think:

git commit -a -m "WIP"
git pull --rebase
# Deal with conflicts
git reset HEAD~1

@stas00
Copy link
Author

stas00 commented Aug 23, 2018

Nope:

$ git commit -a -m "WIP"
[pull-merge c9ac73f] WIP
 23 files changed, 14292 deletions(-)
[...]

$ git pull --rebase origin master
From github.com:stas00/fastai_v1
 * branch            master     -> FETCH_HEAD
First, rewinding head to replay your work on top of it...
Applying: WIP
Using index info to reconstruct a base tree...
M       dev_nb/004_callbacks.ipynb
Falling back to patching base and 3-way merge...
error: Your local changes to the following files would be overwritten by merge:
[...]

Please commit your changes or stash them before you merge.
Aborting
error: Failed to merge in the changes.
Patch failed at 0001 WIP
Use 'git am --show-current-patch' to see the failed patch

Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".

@vidartf
Copy link
Collaborator

vidartf commented Sep 3, 2018

Closing as this isn't really an nbdime issue. If you end up with any suggestions for changes to nbdime behavior to accommodate what you are trying to do, I'd be happy to reopen it!

@vidartf vidartf closed this as completed Sep 3, 2018
@vidartf
Copy link
Collaborator

vidartf commented Sep 3, 2018

PS: Did you open an issue on nbstripout? If so, would you mind cross-referencing it (linking it) here?

@stas00
Copy link
Author

stas00 commented Sep 4, 2018

Not yet. I re-implemented nbstripout and I haven't re-encountered this issue so far. If I do, I will certainly cross-post any new developments here.

@kynan
Copy link
Contributor

kynan commented Nov 10, 2019

FYI, we're discussing adding a dedicated nbstripout command to help with the disable filter - stash - pull - unstash - resolve - enable filter workflow in kynan/nbstripout#108.

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

No branches or pull requests

3 participants