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

Make review of Jupyter notebooks more convenient #9

Open
yvonnefroehlich opened this issue Nov 9, 2024 · 5 comments
Open

Make review of Jupyter notebooks more convenient #9

yvonnefroehlich opened this issue Nov 9, 2024 · 5 comments
Labels
question Further information is requested

Comments

@yvonnefroehlich
Copy link
Member

Reviewing Jupyter notebooks directly on GitHub is not very convenient. GitNotebooks can make the review process easier. As an example, I use PR #4

GitHub: https://github.com/GenericMappingTools/agu24workshop/pull/4/files
gitnotebooks: https://gitnotebooks.com/GenericMappingTools/agu24workshop/pull/4

Maybe this is helpful for use. Feel free to suggest any other tool which you think can be useful.

(modified from issue GenericMappingTools/pygmt-paper-figures#3)

@yvonnefroehlich yvonnefroehlich added the question Further information is requested label Nov 9, 2024
@weiji14
Copy link
Member

weiji14 commented Nov 22, 2024

Hmm, I'm having some trouble putting a review in with gitnotebooks. Am getting some error about Unprocessable Entity: "Pull request review thread path diff too large and Pull request review thread diff hunk can't be blank"

image

Have you seen this before @yvonnefroehlich? I managed to use GitNotebooks for another repo recently, but not sure why I'm getting this error here.

@yvonnefroehlich
Copy link
Member Author

yvonnefroehlich commented Nov 22, 2024

Hm. Yes, I also get this error and sofar I could not figure out why this is.
And no, so far I did not face this issue in other repos (even without having writting permissions).

@yvonnefroehlich
Copy link
Member Author

For me, it works to comment on PR #4 (#4 (comment)) and PR #10 (#10 (comment)) and but not on PR #8 via GitNotebooks. The PRs #4 and #8 were opened by me, PR #10 not. For the PRs #4 and #10, it's possible for me to display the diffs directly on GitHub (e.g., https://github.com/GenericMappingTools/agu24workshop/pull/4/files), but for PR #8 I get the information "1,463 additions, 0 deletions not shown because the diff is too large. Please use a local Git client to view these changes." (https://github.com/GenericMappingTools/agu24workshop/pull/8/files). So, I am wondering if also GitNotebooks has a limitation for the amount of diffs supported in one PR?

@yvonnefroehlich
Copy link
Member Author

yvonnefroehlich commented Nov 23, 2024

So, I think I figured out the reason for this issue: It's related to the images shown within the JN, which are displayed as super long strings (diffs) when the JN is displayed as plain text. For testing, I out-commented Figure.show within the JN in PR #8 (334ef23). After this, I am able to submit review comments via GitNotebooks (#8 (comment)). When adding Figure.show back (44a7bf4), I am again not able to submit a review via GitNotebooks.

Edit:
Temporary for the JN of PR #8, only the finale image is displayed (41116f8). For me, it is now possible to submit a review via GitNotebooks.

Edit:
Now, all images are displayed within this JN.

@yvonnefroehlich
Copy link
Member Author

Maybe it works, after running "Clear output of All Cells", then the large diffs related to the output images are ago.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants