Skip to content

Add illustration to help understand the code that follows - #1097

Open
unode wants to merge 1 commit into
swcarpentry:mainfrom
unode:rectangle_normalization_illustration
Open

unode wants to merge 1 commit into
swcarpentry:mainfrom
unode:rectangle_normalization_illustration

Conversation

@unode

@unode unode commented Feb 27, 2025

Copy link
Copy Markdown
Contributor

When trying to explain the code flow of the rectangle normalization, in a course I quickly put together this illustration which helped visualizing the two execution paths of the code.

Hereby contributing it to the lesson.

@github-actions

github-actions Bot commented Feb 27, 2025

Copy link
Copy Markdown
Contributor

Thank you!

Thank you for your pull request 😃

🤖 This automated message can help you check the rendered files in your submission for clarity. If you have any questions, please feel free to open an issue in {sandpaper}.

If you have files that automatically render output (e.g. R Markdown), then you should check for the following:

  • 🎯 correct output
  • 🖼️ correct figures
  • ❓ new warnings
  • ‼️ new errors

Rendered Changes

🔍 Inspect the changes: https://github.com/swcarpentry/python-novice-inflammation/compare/md-outputs..md-outputs-PR-1097

The following changes were observed in the rendered markdown documents:

 10-defensive.md                       |   3 +
 fig/rectangle_normalization.svg (new) | 255 ++++++++++++++++++++++++++++++++++
 md5sum.txt                            |   2 +-
 3 files changed, 259 insertions(+), 1 deletion(-)
What does this mean?

If you have source files that require output and figures to be generated (e.g. R Markdown), then it is important to make sure the generated figures and output are reproducible.

This output provides a way for you to inspect the output in a diff-friendly manner so that it's easy to see the changes that occur due to new software versions or randomisation.

⏱️ Updated at 2025-02-27 10:04:23 +0000

@unode

unode commented Feb 27, 2025

Copy link
Copy Markdown
Contributor Author

This helped also to identify the logical error reported in #1096

github-actions Bot pushed a commit that referenced this pull request Feb 27, 2025
@unode

unode commented Apr 28, 2026

Copy link
Copy Markdown
Contributor Author

Checking if this contribution is still pending consideration or if it has been rejected.

@noatgnu

noatgnu commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Hello @unode , sorry for taking so long to review this. I agree that this part needs an illustration to better demonstrate the function. However, since it is a normalizing function, I would suggest making the two different rectangles becoming the same type of rectangle after normalization. As current, it is slightly confusing to have two arrows pointing from the same rectangle into two different rectangles. Another suggestion is adding a coordinate system to each rectangle to make the x0, x1 and y0, and y1 clearer for the learners.

Thank you for your contributions.

@unode

unode commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

Hi @noatgnu, the intention with the two rectangles is to give a visual indication which of the "sides" needs to be scaled based on the condition. Notice that the condition in the center of the rectangles is different between the two.

Either way, from the discussion in #1096 and the #1096 (review) comment, it seems there are other issues with this section.

From personal experience delivering this lesson, the defensive programming section has frequently been found to be difficult for learners to understand. Part of it, I think, comes from the not so simple example and the somewhat unexpected intentional error left in the assertions section.
For the learner it is meant to be a ha-ha! moment but from my experience it doesn't work due to the complexity of the example.

Either way, the suggestion was merely to try to help with this section that has always been difficult to teach. I've been more successful skipping the "Assertions" section altogether and directly starting with "Test-Driven Development".


Going a little off-topic, others have also raised concerns due to the fact that assert statements can be disabled at runtime. The risks are well understood and try/except or if/.../else are generally preferred, even if they are more verbose.
This is especially true for libraries where exceptions should be documented. Handling AssertionError is generally difficult from other code. You know an error happened but to tell why or to know if you can recover from it, you need to parse the assertion message. Fine for humans but non-trivial in code.

This branch has not been deployed

No deployments
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