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

Notebooks - Diff indicator shown despite same output #135820

Closed
claudiaregio opened this issue Oct 26, 2021 · 14 comments
Closed

Notebooks - Diff indicator shown despite same output #135820

claudiaregio opened this issue Oct 26, 2021 · 14 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug notebook-diff verified Verification succeeded
Milestone

Comments

@claudiaregio
Copy link

  • VS Code Version: 1.62.0-insider
  • OS Version: WIndows

Diff color indicators on output that is the same

image

@rebornix rebornix added the info-needed Issue requires more information from poster label Oct 26, 2021
@rebornix
Copy link
Member

@claudiaregio can you please share the modified file content? Also do you see any change when you open the diff editor in text diff?

@claudiaregio
Copy link
Author

claudiaregio commented Oct 26, 2021

Original Notebook + env required to run: https://github.com/claudiaregio/data-science
Titanic_mod.zip

This is what I'm seeing for the text diff
image

@rebornix
Copy link
Member

@claudiaregio is the screenshot the only change in the document?

@claudiaregio
Copy link
Author

There are other changes in the document, since all I do is run I believe it's flagging the output ID changing

@burkeholland
Copy link

burkeholland commented Nov 1, 2021

Hit this in the Golden Scenario today. I am unable to tell from the diff what has changed and what has not as the output looks identical to me. Perhaps this is a matter of just knowing that the underlying JSON changed, but I found it confusing as I was looking for differences in places where there really is none of material importance.

whats-changed.mp4

@rebornix
Copy link
Member

rebornix commented Nov 2, 2021

Thanks for the file provided above and the screen recording!

Dig into this and found it interesting, is there an output change? Yes, but it is not anything that one can tell from the Diffing. What are the changes?

  • The output contains metadata, which includes the execution count, thus they are different on different runs. Not sure if they are saved to disk, but our diff view doesn't really tell you if it's output metadata change, or output value change. This is something we should improve in the diff editor
  • If we then compare the output, the sequence of text/html/`text/plaintext' is flipped. This is when you see no change in the rich diff editor, but you see an entry in SCM cc @roblourens @DonJayamanne

@roblourens
Copy link
Member

Maybe they need to be sorted along with other keys in #129435

@roblourens
Copy link
Member

roblourens commented Nov 2, 2021

We already sort all keys in the notebook when serializing, so the order of the mimetypes should have been sorted. Which order did you see it in? Maybe it started out wrong? In that case Jupyter would have done the same thing.

@tanhakabir
Copy link
Contributor

+1 from golden scenario run by me

@github-actions github-actions bot locked and limited conversation to collaborators Jan 1, 2022
@greazer greazer added the bug Issue identified by VS Code Team member as probable bug label Feb 4, 2022
@greazer
Copy link
Member

greazer commented Feb 4, 2022

Reopening this as it contains a lot of extra info and it's been repro'd and re-entered at least 2 other times. We really should do something to address the problem.

@greazer greazer reopened this Feb 4, 2022
@greazer greazer removed the info-needed Issue requires more information from poster label Feb 4, 2022
@greazer greazer added this to the February 2022 milestone Feb 4, 2022
@roblourens
Copy link
Member

Could you link the dupe issues?

@rebornix
Copy link
Member

rebornix commented Mar 7, 2022

Should be fixed in today's Insiders.

@rebornix rebornix closed this as completed Mar 7, 2022
@joyceerhl joyceerhl added the verified Verification succeeded label Mar 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug notebook-diff verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

8 participants
@roblourens @burkeholland @rebornix @greazer @tanhakabir @claudiaregio @joyceerhl and others