python-novice-inflammation icon indicating copy to clipboard operation
python-novice-inflammation copied to clipboard

A concrete example of moving from assertions to exceptions

Open DamienIrving opened this issue 2 years ago • 12 comments

Hi! I'm the lead maintainer of the Data Carpentry Python for Atmosphere and Ocean Science (PyAOS) lessons.

When I first wrote the PyAOS lessons a few years ago, I borrowed heavily from the Software Carpentry lessons. My defensive programming lesson was almost a straight copy of the corresponding python-novice-inflammation lesson. However, last year I received feedback (https://github.com/carpentries-lab/python-aos-lesson/issues/36) similar to #622, #773 and #874, suggesting that in most cases raising exceptions is a better approach to using assertions. I therefore just went through the process of re-writing our defensive programming lesson to address that feedback:

https://carpentries-lab.github.io/python-aos-lesson/08-defensive/index.html

Instead of teaching assertions and then test-driven development, we now teach the following (quoted directly from the lesson):

The first step toward getting the right answers from our programs is to assume that mistakes will happen and to guard against them. This is called defensive programming, and there are a number of tools and approaches at our disposal for doing this. Broadly speaking, we can:

  • raise and handle errors to check and respond to program inputs
  • use assertions to make sure nothing crazy or unexpected has happened
  • write unit tests to make sure each component of our program produces expected outputs
  • use a logging framework to report on program activity

In this lesson, we’ll look at how error handling, assertions and logging can make the unit conversion in our program more reliable, and we’ll provide links to further information on unit testing.

If people think our new PyAOS defensive programming lesson looks like a good approach to addressing the concerns in #622, #773 and #874, I'd be happy to submit a PR that adapts the content in our PyAOS lesson to fit with the Software Carpentry defensive programming lesson?

DamienIrving avatar Mar 04 '22 05:03 DamienIrving

👍 as a lurker from the gapminder lesson I think this is a solid contribution, but I would recommend including conditionals as well to check for invalid inputs as a first line of defense before exceptions. In grad school I was taught to avoid using exceptions for flow control, i.e., as guards to dictate which branch a piece of code should flow into, but this was primarily for performance reasons.

But I think it's a useful distinction to leave exceptions for exceptional cases (i.e., if the filesystem cannot be written to, the network is down or unreachable, etc) vs invalid user input or other recoverable errors.

These issues are also well articulated in https://stackoverflow.com/a/1835844/93370

alee avatar Mar 04 '22 05:03 alee

Hey Damien!

Some time ago maintainers of this lesson (myself included) had a lengthy discussion about it and decided that changing assertions to exceptions is the path we want to take. We mentioned it in some issues and PRs here and there. So, if you're willing to make such a contribution -- it would be immensely appreciated.

hey @alee 👋🏻 good to see you! I'm in Arizona now, so we can, perhaps, meet some time :)

maxim-belkin avatar Mar 04 '22 05:03 maxim-belkin

Tagging @swcarpentry/curriculum-advisors in case they would like to comment on this. (According to our CAC consultation rubric, this does not require CAC approval but it seems like the kind of topic the Curriculum Advisors might have a valuable opinion on.)

tobyhodges avatar Mar 04 '22 09:03 tobyhodges

Thanks for the info, Toby! This change can be interpreted as removal of one episode and addition of a new one, so it looks like it does require an OK from the CAC.

maxim-belkin avatar Mar 04 '22 10:03 maxim-belkin

Thanks for tagging us!

I, as one member of the CAC, agree with your interpretation of the rubric, and think this change falls under "Issues that may benefit from Maintainers consulting with the CAC, but over which Maintainers retain authority".

I'm glad to hear that the maintainers already have consensus on how they'd like to evolve this lesson.

We may bring it up during our first meeting (which will be scheduled for sometime in the next 6 weeks), to discuss informationally. My understanding is that maintainers are welcome at CAC meetings 🙂. If we do discuss it as an agenda item we'll also report back on any additional thoughts.

bsmith89 avatar Mar 04 '22 18:03 bsmith89

Thank @bsmith89 it sounds like you don't have any concerns about @DamienIrving working up a pull request to make these changes before the CAC has a chance to meet and discuss this. If the pull request has been merged already by the time the Curriculum Advisors have that conversation, please ask CAC members to open a new issue or pull request with any changes or suggestions they would like to make.

tobyhodges avatar Mar 07 '22 09:03 tobyhodges

Great. Thanks, everyone.

I've started a draft pull request at #981 and I'll let you know when it's ready for review (hopefully in the next week or two).

DamienIrving avatar Mar 10 '22 01:03 DamienIrving

#981 is now ready for review if people want to take a look

DamienIrving avatar Mar 17 '22 03:03 DamienIrving

@maxim-belkin, @tobyhodges Is there anything I can do to help progress this? The PR is ready to go #981

DamienIrving avatar Aug 01 '22 07:08 DamienIrving

Hi, @DamienIrving. Apologies for dropping the ball here. I remember looking at the PR and, IIRC, I wanted to make quite a few comments, so decided to postpone this until I had enough time (I was changing jobs around that time). Unfortunately, I'm no longer a maintainer so I'm going to yield the right of reviewing the PR to current maintainers.

Basically, the main ask / request from me was going to be to try to rewrite the text to avoid using jargon and saying that something is simple. Example of these would be:

Signal errors by raising exceptions.

Raise and handle errors

One of the most simple tools in the defensive programming toolkit is the raise keyword

We'd have to introduce learners to the jargon before we can use it ("signaling an error", or "raising an exception") and I can't say that raise falls into the category of "simple tools". So, it would be great if you could review (and update) your PR from the point of view of explaining this to someone who knows very little about programming and is not familiar with jargon.

maxim-belkin avatar Aug 01 '22 18:08 maxim-belkin

Thanks, @maxim-belkin. I've had a go at removing jargon (or at least explaining it before using it) and also removing any suggestion that this stuff is simple.

@noatgnu, @ineelhere, @tobyhodges - is there anything else I can do to progress the PR at #981?

DamienIrving avatar Oct 17 '22 03:10 DamienIrving

Relevant discussion from @swcarpentry/curriculum-advisors in swcarpentry/curriculum-advisors#1.

bsmith89 avatar Dec 29 '22 19:12 bsmith89