Skip to content

SP 2407: NB 103_8 matplotlib#91

Merged
shenmingfu merged 9 commits into
mainfrom
tickets/SP-2407
Nov 28, 2025
Merged

SP 2407: NB 103_8 matplotlib#91
shenmingfu merged 9 commits into
mainfrom
tickets/SP-2407

Conversation

@shenmingfu
Copy link
Copy Markdown
Contributor

No description provided.

@review-notebook-app
Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Copy Markdown
Contributor

@MelissaGraham MelissaGraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a list of requested changes in the Jira ticket.

@shenmingfu
Copy link
Copy Markdown
Contributor Author

@MelissaGraham Thanks. I've adjusted the notebook and addressed your comments.

@shenmingfu
Copy link
Copy Markdown
Contributor Author

I noticed in a previous commit I edited 103_5 by accident. I tried "Interactive Rebase" but it didn't work. So I used "Git Revert" and it worked.

@MelissaGraham MelissaGraham self-requested a review November 25, 2025 22:25
Copy link
Copy Markdown
Contributor

@MelissaGraham MelissaGraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A demo of how to see the docs for display.scale or display.image is tucked into the end of the subsection on masks, but it would be more relevant at the start of section 2 or 2.1.

Subsections headers in sections 2 and 3 are missing a period after the number (like as in the subsections of section 1).

Section 3 header can just be "Display an image with imshow", the matplotlib isn't necessary and then this would better match the section 2 header.

Looks like you'll have to update with rebase but then please go ahead and merge, thanks Shenming.

@shenmingfu
Copy link
Copy Markdown
Contributor Author

@MelissaGraham Thank you very much. I've addressed your comments and updated the notebook, and I also did the rebase. After the checks I will press the merge button.

@shenmingfu shenmingfu merged commit 17cd2c7 into main Nov 28, 2025
2 checks passed
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

Successfully merging this pull request may close these issues.

2 participants