-
-
Notifications
You must be signed in to change notification settings - Fork 783
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
Using Assert as input validation is wrong and needs to be changed #1084
Comments
Hi @dhvalden, |
Dear @AlbertoImg, Of course! No worries. But just to reiterate. The use of |
Dear @AlbertoImg, I just created the promised pull request. |
Hi @dhvalden, thanks for the pull request! we will look into it with the curriculum advisors. |
Answering as a member of the CAC, but not representing the rest of the CAC. I can agree that one should not use assert as input validation (although not all examples of assertions in the code of this episode are input validation). However these changes are much, much wider than that, and I think they would all need to be discussed. Just a few examples:
In summary: one thing is removing the use of assert as input validation, one thing is completely rewriting the episode. |
Hey! @martinosorb To comment on your points:
Finally, correct input validation and correct error handling is a much more "beginner need" than test driven development. The former will be useful all the time in simple scripts, whereas the latter becomes important in a larger interconnected project, like a library or app. Even the Official Python Tutorial thinks this way, with Errors and Exceptions handling being the 8th lesson https://docs.python.org/3/tutorial/errors.html whereas |
I can see both side's points here but I'm also leaning more towards @dhvalden and want to add a few aspects to the discussion.
To summarise, that section always felt a bit off in my teaching, too. Both concepts are important but the current lecture doesn't do either of them a favour with using bad practice for input validation and leaving a lot of room for misinterpretation concerning TDD. I think focusing on one would benefit the learner. Which one, is for a bigger audience to decide. |
What is the problem?
The problem has been reported several times already, so I'm late to the party (here fore example #979).
To recap,
assert
keyword is not to be used as proposed in the example of the episode of Defensive Programming. The reason has been pointed out several times with various degrees of detail, but it boils down to that is basically a debug and testing phase tool, that can be bypassed at runtime by the interpreter. Python documentation states that basically evaluates to:The episode of Defensive Programming focuses mainly on input validation. Input validation needs to stop the program every time an incorrect input is passed.
if raise
blocks are the standard and canonical way of doing this. For an example see this function in the scipy repository:https://github.com/scipy/scipy/blob/v1.14.0/scipy/optimize/_minimize.py#L53-L775
I do not propose to also add the idea of
exceptions
neithertry except
blocks, since a novice user will not need this, I simple propose to replaceassert
blocks withif raise
blocks. I will provide a pull request of this in the coming days.Location of problem (optional)
https://github.com/swcarpentry/python-novice-inflammation/blob/main/episodes/10-defensive.md
The text was updated successfully, but these errors were encountered: