-
Notifications
You must be signed in to change notification settings - Fork 159
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
Support cell IDs #553
Comments
The fresh release seems to have broken a lot of tests (cf. #551). This should be fixed, or at the very least be verified to be a test-only issue. If this affects the current releases, a patch release based on the last release should be cut. |
My guess is that it's a test-only issue and that the added field doesn't factor into user-facing output other than possibly seeing the cell ids. I'd suggest: step 1, ignore cell id and make a release with unchanged behavior that is aware of cell ids, as if it were any other opaque metadata field. Since cell IDs are hidden from the user, I think showing them in diffs should also be done at the changed-metadata level, maybe hidden by default. Then I think we can explore using cell ids as highest priority for alignment. Since that would potentially simplify things, I think it's a good thing to try, and eliminates cell id conflict issues (right?). Obviously, a contrived example can be created to generate an equivalent-content notebook with the same ids in different order where a big diff would be a bad experience, but I don't think that's likely to happen, at least not often. Then I'd say only downgrade cell id precedence again if this proves to be a poor experience in practice, and only at that point are cell id conflicts needed to be resolved, where I think a typical "use theirs/use ours" strategy followed by an assert-uniqueness pass on the whole cell list to regenerate later occurrences of any duplicate cell ids should suffice. |
I agree with the plan 👍 Here are some cases that should be considered as part of the exploration for using cell ids as the highest priority:
|
What should happen for merge action in the initial phase of the plan? |
Thanks, I'll do my very best to get it out next week! |
This was only partially resolved by #566, but GH was a little over eager 😄 |
Hi @vidartf and @krassowski is there any planned work in this area? |
When time permits. But if you or someone else want to help push this, please do so (or reach out to help coordinate it). |
See recent JEP and implementation:
For nbdime, we then need to consider whether we should use IDs as a high level indicator of cell identity when diffing. Questions that need to be resolved are:
The text was updated successfully, but these errors were encountered: