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

A concrete example of moving from assertions to exceptions #979

Open
DamienIrving opened this issue Mar 4, 2022 · 12 comments
Open

A concrete example of moving from assertions to exceptions #979

DamienIrving opened this issue Mar 4, 2022 · 12 comments

Comments

@DamienIrving
Copy link
Contributor

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 (carpentries-lab/python-aos-lesson#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?

@alee
Copy link
Member

alee commented Mar 4, 2022

👍 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

@maxim-belkin
Copy link
Contributor

maxim-belkin commented Mar 4, 2022

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 :)

@tobyhodges
Copy link
Member

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.)

@maxim-belkin
Copy link
Contributor

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.

@bsmith89
Copy link

bsmith89 commented Mar 4, 2022

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.

@tobyhodges
Copy link
Member

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.

@DamienIrving
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@DamienIrving
Copy link
Contributor Author

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

@maxim-belkin
Copy link
Contributor

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.

@DamienIrving
Copy link
Contributor Author

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?

@bsmith89
Copy link

bsmith89 commented Dec 29, 2022

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

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 a pull request may close this issue.

5 participants