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

Update 08-defensive.md #693

Open
wants to merge 1 commit into
base: gh-pages
from

Conversation

@wachiuphd
Copy link

commented Aug 16, 2019

As written, the assertion that checks that each value can be converted to an integer will fail if one of the elements is 0, since int(0) is 0 which is treated as False. Zero is a legitimate value for a number of cars.

Update 08-defensive.md
As written, the assertion that checks that each value can be converted to an integer will fail if one of the elements is 0, since int(0) is 0 which is treated as False.  Zero is a legitimate value for a number of cars.
@maxim-belkin

This comment has been minimized.

Copy link
Contributor

commented Aug 26, 2019

Thank you, @wachiuphd, for pointing out the problem!

assert int(0) indeed fails because 0 is treated as False. There is another problem with assert int(element)too: int('123') is True whereas int('0') is still False.

However, we don't talk about isinstance in our lesson at the present time and introducing it in a challenge without any explanation doesn't seem like the right thing to do.

How about changing the assert statement to the following?

assert int(element) or element == 0

@maxim-belkin maxim-belkin requested a review from annefou Aug 26, 2019

@maxim-belkin maxim-belkin self-assigned this Aug 26, 2019

@wachiuphd

This comment has been minimized.

Copy link
Author

commented Aug 27, 2019

The other problem with this is that it doesn't really demonstrate "assert," which requires a True or False. For instance, the code

for element in [1,2,3.2,"a"]:
        assert int(element) or element == 0

gives an error totally unrelated to assert

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-3-06c1ff32006e> in <module>()
      1 for element in [1,2,3.2,"a"]:
----> 2         assert int(element) or element == 0

ValueError: invalid literal for int() with base 10: 'a'

This error occurs even before the "assert" part is called. You would get the same error from code without assert

for element in [1,2,3.2,"a"]:
        print(int(element))
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-5-2a1a80dabb96> in <module>()
      1 for element in [1,2,3.2,"a"]:
----> 2         print(int(element))

ValueError: invalid literal for int() with base 10: 'a'

edited by @maxim-belkin to stylize code/error blocks.

@maxim-belkin

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2019

The other problem with this is that it doesn't really demonstrate "assert," which requires a True or False. For instance, the code

for element in [1,2,3.2,"a"]:
        assert int(element) or element == 0

gives an error totally unrelated to assert

Yes and no.
Yes: this error is not raised by the assert statement.
No: this error is related to the assert in a sense that non-integers is what we want to catch.

Obviously, try/except block is what we need here, not assert, but this is a bigger change.

Now, going back to the current episode: shall we do type(element) = int? I'm hesitant to add isinstance here: we either have to introduce it early in the lesson or not talk about it at all. Moreover, the two things that isinstance brings compared to the direct type check are (a) ability to specify more than one acceptable type (b) inheritance of classes. The latter one would, actually, cause a "problem" for us since Booleans are a subclass of integers in Python.

>>> isinstance(True, int)
True
@wachiuphd

This comment has been minimized.

Copy link
Author

commented Aug 28, 2019

Looks like a good solution. I assume you mean type(element) == int (== not =)?

Of course, that means a float such as 3.2 will assert as false (whereas int(3.2) will convert to an int). But this is probably better anyway.

@maxim-belkin

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2019

Looks like a good solution. I assume you mean type(element) == int (== not =)?

Yes, ==. Apologies for the typo.

Of course, that means a float such as 3.2 will assert as false (whereas int(3.2) will convert to an int). But this is probably better anyway.

I wonder what 3.2 cars look like... 🤔 So, yeah: disallowing floats should be fine.

@maxim-belkin

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2019

Forgot to mention: please make sure to change output cells in the exercise too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.