Feedback Module 6 - Githubissues

@mgelbart Here is the first draft of Module 6! it is available here. There is also assignment 6 that is done too ( here).

I plan to organize and start content for Module 7 Monday and Tuesday!

Posting what I've done so far (Ex1) in case you want to review it before today's meeting. I'll try not to forget the rest this time!

[x] 1.1: "Dry" -> "DRY"
[x] 1.3: add colon after the text
[x] 1.4: "are" -> "as"
[x] 1.4: I'm seeing major improvement from a few months ago. Nicely done.
[x] 1.5: "that it was made" -> "where it was made"
[x] 1.6: comma goes after "that" instead of before;; "it's not a good to have" -> "it's not good practice to have"
[x] 1.7: "Most people from Vancouver recognized it," -> "Many people from Vancouver recognize it; " ;; "city of Vancouver" -> "City of Vancouver" ;; "Starbuck" -> "Starbucks"
[x] 1.8: Capital "W";; I would also mention that it also uses the local copy if it comes in as an argument def squares_a_list(numerical_list, a_new_variable):
[x] 1.10: These are very dangerous waters. Also this is technically true, it's also misleading. Because if global_var is a list then you can modify its elements inside of the function, even if you can't modify "the list itself". I think we might be better off saying the situation is complicated and we recommend not trying to do this. Update: oh, now I look at 1.11. Hah. Ok soo... I think we'd better drop 1.10 and go straight to 1.11. I think 1.10 just muddies the waters. Then we'd also need to remove "Since we did not use the assignment operator, we were able to make a modification to the global variable. " in 1.12. Or, if we really want 1.10, I'd at least move it to the end or something.
[x] 1.12: don't need capital letters for side effect.
[x] 1.13: "Remember our cereal dataframe, w" -> "Remember our cereal dataframe? W"
[ ] 1.14: not sure if this is coming up, but a bunch of the pandas commands have an inplace option, which we recommend not using, but which we could mention here. E.g. https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.DataFrame.drop.html, https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.DataFrame.sort_values.html, etc. Update: around 1.17 seems like a great time for this.
[x] 1.15: watch capitalization Cold vs. cold;; return should be in code font;;
[x] 1.16: you didn't show the calling of the function - should show it!
[x] 1.17: not sure if this is coming later, but we should say that if you have side effects, they need to be clearly documented in the docstring (have we taught docstrings yet? if not, do we want to reorder?);; have they seen copy before?
[x] 2.1: I got this wrong facepalm
[x] 2.2: I got this wrong also. But are you sure?? I think it might be better to remove this, per my comments about 1.10 above.
[x] 3.2: I don't think we need Error as an option here. How about Both and Neither instead?
[x] 3.3: This is OK but not ideal. Here's my concern: if you have a function f that calls a function g, then g could mess with a variable from f. It's a side effect, but the variable is not a global variable. It's just a variable from outside the scope of g. Could we maybe tighten up the wording of the question?
[x] 4: consider splitting this into 2 code blocks, so they can more easily see the output of the provided code- not possible currently because of all the ___
[x] The docker stuff is still pretty slow, oh well.
[x] I'm having a problem with docker. For ex 4, I press run code and get an error. Then I deleted all the ___ and pressed "run code" again but nothing happens! Does this happen to you as well??
[x] 5.2: "When we were learned functions" grammar
[x] 5.4: again, clear improvement from a while back
[x] 5.5: "Python allows for optional arguments that can take a default value." -> this is introducing two ideas at the same time: the default aspect and the optional aspect. I know they are really the same. But to keep their minds clear, I'd instead just talk about the default value first. And then slightly later mention the word "optional". So something like "Python allows for default values in the event that an argument is not provided to the function.";; not sure if this is coming later, but at some point we should clarify that you can call it with exponent_a_list(numbers, 5) or exponent_a_list(numbers, exponent=5)
[x] 5.8: "Our function doesn’t recognize the input arguments and an error occurs." -> "Our function doesn’t recognize the input arguments and an error occurs because the two arguments are being swapped - it thinks 5 is the list and [2, 3, 5] is the exponent.";; ok so this is again teaching 2 things at the same time; as mentioned above, we need to first teach exponent_a_list(numbers, exponent=5), then talk about how it's OK to call exponent_a_list(numerical_list=numbers, exponent=5) and then talk about the ordering issue that we see here.
[x] 6: nice!
[x] 7.1: nice!
[x] "that that" ;; there's a slash at the end of the correct-answer-text
[x] 8: this is not very plausible because the conversion rate is fixed and would never change. How about doing some physics calculation, like the distance a ball goes if you throw it at a given angle and initial velocity, and then the optional parameter could be g, the acceleration due to gravity. We'd normally set g=9.8 (m/s^2) but hey, if you wanted to use this for the moon, you might need to change g?? dunno, maybe this is too involved in the physics, but something more plausible would be nice.

Here is 9-16, I'll have to do the rest tomorrow (or Thursday at the latest!):

[x] 9.2: rewrite intro: Functions can get very complicated, so it's not always obvious what they do just from looking at the name, arguments, or code. Therefore, people like to explain what the function does. The standard format for doing this is called a docstring. A docstring a literal string that comes directly after the function def and documents the function's purpose and usage.;; period after "things"
[x] 9.4: remove
[x] 9.5: "The docstring style that is the most elaborate and informative is called NumPy style" -> "There are several styles of docstrings; the one we'll use is called the NumPy style";; "Creates a new list containing the square values of the input list. " -> "Squared every element in a list."
[x] 9.6: "The NumPy style docstring is easier to read and extract information from in comparison to the ReStructuredText format." -> delete;; even before this, maybe clarify the """ which they've seen but maybe forgotten. and also clarify that this doesn't change how the code runs, the docstring is for human consumption (either looking right at it or though some help query)
[x] 9.8: I think we can cut this
[x] 9.9: I'm not sure, pls check if the default part needs to be in more of a standardized format in the numpy style, e.g. "default = 2" or "default : 2"
[x] 9.10: we should clarify that part of the use is people who are directly looking at the code, not just this ? type thing
[x] 11: I struggled with this one! (and eventually got it wrong, picked A) . Could you maybe make the problem a bit more obvious, aside from just swapping those two? Also, is there supposed to be a colon after the type in the returns section?
[x] 12: let's indent the ___
[x] Just an idea for the docstring slides, maybe it would be fun to show only the code for a complicated function (maybe something online, depending on license issues?) and say "we have no clue what this does" and then on the next slide show only the docstring, and suddenly it's super clear.
[x] 13.2: "programmers now have a method that is structured around the assumption that errors will occur." -> "we have a way of defending our functions against potential issues."
[x] 13.4: "Exceptions disrupt the regular execution of your code.";; "When we raise an Exception we are forcing our own error with our own message if the condition results in a True value." -> this is misleading/incorrect, the Exceptions are separate from the if statements we're using them with. Let's just delete the "if the condition results in a True value.";; have they seen isinstance before?
[x] 13.5: Argh, I know I keep contradicting myself, but I wonder if we should have just gone with if type(numerical_list) != list instead, so they didn't have to learn something new here. Let's discuss at the next meeting. ;; int should be list;; change "If the condition is True, what the problem is" to "raise an Exception (throw an error) with this message";; no space in "straight forward"
[x] 13.6: "which checks if the first argument is of the same data type as the second argument." -> not quite true, maybe change to "which checks if the first argument is of the data type given by the second argument (or, technically, a similar type)."
[x] 13.7: How about a more realistic check, that makes sure the numbers in the list are >= 0 (to avoid stuff like sqrt(-1)). You can use a for loop here outside the if.
[x] 13.8: oh, here you go. Ok, let's delete 13.7 and then change 13.8 to be >=0 instead of type int. Let's use a regular for loop instead of a comprehension for simplicity; this also avoids the need for all.
[x] 13.9: "We discussed the conditional part of an exception" -> misleading, see above;; "Exception is just one of the many different exception types possible. The exception in our previous examples is a more generic type which is defined by the person who writes code of the function. This isn’t all-encompassing as other Exceptions are likely better suited for the raises we made." -> "The exception type called Exception is a generic, catch-all exception type. There are also more more specific types of exceptions; for example, you may have encountered ValueError or a TypeError at some point." there's a confusion here between what "an exception" is. Is this an exception? Exception("You are not using a list for the numerical_list input.") I guess so. But in my head I immediately think of different types of exceptions if someone talks about different exceptions. So maybe we shouldn't muddy the waters by saying that Exception("A") and Exception("B") are two different exceptions.
[x] 13.11: will need updating (or removing) per comments above
[x] 13.12: I would delete, could be confusing
[x] 13.13: remove the #Exception in the URL
[x] 14: "Why would we want to raise an exception?" -> "Why would we want to write code that raises an exception?"
[x] 15: this is a lot, maybe reduce down to only 2 possible answers?
[x] 16: "(Disclaimer: Much more than these two measurements are needed to determine if someone is healthy)" -> I think this makes things worse rather than better. it draws attention to the fact that this is about health.

[x] 17.2: I would say "These questions can be answered by using something called units tests. We'll be implementing unit tests in Python using assert statements." assert statements are just one way of implementing this, but the big idea is the unit tests.
[x] 17.3: "Unlike anException which force an error when the condition evaluates to True, an assert statement cause our program to fail if the condition evaluates to False." -> same issue discussed above, Exceptions are separate from conditionals; to me, the distinction is that assert basically blows up your program, whereas in advanced programming Exceptions my be handled in various ways (this is the failing gracefully you mentioned earlier). Also, (just FYI, not for including in the course), it is possible to disable all asserts, which is sometimes done for speed, if the tests are slow to run. I remember doing this in C/C++. Anyway, asserts are strictly about debugging the program, whereas Exceptions are more like part of the program, I would say. This is also why Exceptions are described in the docstring whereas asserts would not be. I think your use of the phrase "sanity checks" is appropriate. ;; Let's remove print('Will this line execute?') , or put it in later . At first let's not have any distractions from the assert.
[x] 17.4: Assert -> assert;; be consistent about boolean vs Boolean (better with the capital since Boole was a person);; there's an extra space in " 1 is not equal to 2.";; so what I'd do is make a new slide with

assert 1 == 1 , "1 is not equal to 1." (note your message needs to be changed here)
print('Will this line execute?')

and

assert 1 == 2 , "1 is not equal to 2."
print('Will this line execute?')

[x] 17.5: change ". We recommend" to ":" - I feel too hypocritical saying that because I don't actually do it myself. Tiffany is probably better about it.
[x] In general, I think we need to do a better job of separating the concept (unit tests) from the syntax (assert). It feels a bit muddled right now. Something earlier on would be helpful, like on the first slide, saying we'll first talk about assert and then how we use them in unit tests. Remember, we have the table of contents in our head, and where we are in it, but the learner doesn't have that.
[x] 17.6: "It’s also good to do multiple test to for different list sizes" (multiple grammar issues). ;; we should be consistent here and use isinstance if that's what we're teaching
[x] 17.7: after bad_function is defined, need some short text explaining what's wrong with it;; this is a pretty contrived example, let's think of something more realistic. A slightly more plausible one (though still not great) could be:

def bad_function(numerical_list, exponent=2):
    new_exponent_list = [numerical_list[0] ** exponent] # seed list with first element
    for number in numerical_list[1:]:
        new_exponent_list.append(number ** exponent)
    return new_exponent_list

which will fail for an empty list.

[x] 17.8: "the correct answer is infinity" -> "we might want the function to output infinity".
[x] 17.9: "(Note that at this point it will not have been written out and it would just be an idea)" -> (Note: if we're using test-driven development then this function will just be an idea, not just completed code.)
[x] 17.10: "my" -> "";; this could be explain a bit better. we should say clearly that we need to invent/synthesize some made-up data so that we can write unit tests.
[x] 17.11: I'm a bit confused about this, because this code does not run. Would return list() be better? I would prefer something that runs.
[x] Is this ordering taken from somewhere? As I mentioned above, I don't actually do it this way, but I agree it's good to teach it. If what you have is the agreed upon gold standard, that is fine. If we're just making it up, maybe we can move the docstring earlier. I would have actually thought that would be step 1, right after the stub. Maybe let's discuss this with Tiffany.
[x] 17.13: have they heard the term pseudo-code before?
[x] 17.14: I'd mention that as you work on the code, more and more tests will pass over time.
[x] 18.1: put "not" in bold/italics like 18.2
[x] 18: the format is a bit confusing. How about you show all 4 lines of code, e.g.

assert acronym_it("Hard work pays off") == 'HWPO'
assert acronym_it("Hard work pays off") == 'Hwpo'
assert acronym_it("Hard work pays off") == 'H'
assert acronym_it("Hard work pays off") == str

and then we don't need the ??? at all. Then for the answers you'll just need to repeat the 4 lines like you did for 18.2. And for 18.2 I'd also show the 4 lines.

[x] Also, I don't think we told them that the message is optional for assert, which may confuse them when they get here
[x] 19: missing a space in the last choice;; also, I don't know, this one is a bit subjective. There might be a purpose to those tests. for bad tests, what if you add a docstring to the function that shows the inputs are supposed to be numbers, and then have a silly test like multiply(["a", 5]) == "aaaaa", which passes, but doesn't really test what the function is supposed to be doing?
[x] 20: "ere" -> "err".
[x] 21: do we really need the messages here?;; again, we should decide if we're doing type(x) == or isinstance. I wonder if maybe type(x) == is better. but my problem is an ego/vanity problem, that I don't want to look like I don't know. In such cases I usually pick the way that's better for students but add a little disclaimer like "See isinstance for a more advanced approach."
[x] 22.2: nice.
[x] 22.3: "it decreases the readability, makes code problematic to maintain and makes being consistent becomes more difficult" -> I think the main point is flexibility. By being inflexible, you end up writing more functions and violating DRY, which is what hurts those other things you mentioned. But I agree it also hurts readability- if something is an argument then the docstring explains it, but if it's just a magic number then you have no clue where it came from or what it represents. we should maybe mention the term "magic number" somewhere? E.g. right after "In this function, we “hard-coded” in 2 when we calculated number ** 2." we could add "This is also called a magic number." and leave it at that;;
[x] I think after 22.3 we need to review why the new function is better than the old one - maybe a new slide
[x] 22.6: just a note that we have type(x) == here as well.
[x] 22.4-22.6: I'm not sure sold on this example. I think it might be better to go the other way, exponent_a_list_of_lists and show why that is weird. Or could we go with the example from DSCI 511 instead?
[x] 22.9: needs a bit of work. let's be more specific about the pain points:
- can't access the mean
- what if we don't want the plot
- what if we want the mean but don't have altair installed (maybe too advanced a concept for this course, probably don't mention this)

In particular "what happens if we want to do other calculations with the dataframe?" is a bit vague.

[x] 22.10: not sure if this works here, but it would also be reasonable to show one function calling the other one if you're splitting into two functions.
[x] 22.12: "doesn’t make any sense when we read back our code and" -> delete
[x] 22.14: furthermore, and maybe more importantly than the problem mentioned, is that now the function only works on the cereal data - it's not usable on other data.
[x] 23.2: there is a technical problem that I can't select true/false. But overall I think we can just delete 23, I don't find it that helpful.
[x] 24.1: need an "s" at the end of object
[x] 24.2: rename function to count_the_sevens;; answer 4 is incorrect, here's the message:

Incorrect. Perfect! Let’s make the count variable (in this case the 7) an input argument.

😂

[x] 25: let's provide a docstring for wage_increase and in general let's set a good example by providing docstrings from now on;; I didn't really know what I was supposed to do here, let's make the instruction more explicit. How about we give them the stub and the docstring for the new function? or at least tell them what design practices, I guess just the loop one and the hard-coding one, right?

I would also mention that it also uses the local copy if it comes in as an argument def squares_a_list(numerical_list, a_new_variable)

I'm not sure what you mean, like if

def squares_a_list(numerical_list):
    a_new_variable = "Ta-Da!"
    print( a_new_variable)

    new_squared_list = list()
    for number in numerical_list:
        new_squared_list.append(number ** 2)
    return new_squared_list

and we said numerical_list = [54, 25, 3] from within the function?

1.10: These are very dangerous waters. Also this is technically true, it's also misleading. Because if global_var is a list then you can modify its elements inside of the function, even if you can't modify "the list itself". I think we might be better off saying the situation is complicated and we recommend not trying to do this. Update: oh, now I look at 1.11. Hah. Ok soo... I think we'd better drop 1.10 and go straight to 1.11. I think 1.10 just muddies the waters. Then we'd also need to remove "Since we did not use the assignment operator, we were able to make a modification to the global variable. " in 1.12. Or, if we really want 1.10, I'd at least move it to the end or something.

I just removed 1.10 altogether. That work?

1.16: you didn't show the calling of the function - should show it!

I do show it? It just doesn't produce an output since the function has no return.

1.17: not sure if this is coming later, but we should say that if you have side effects, they need to be clearly documented in the docstring (have we taught docstrings yet? if not, do we want to reorder?)

We tackle docstrings 2 slide decks from now. Should I come back to it in that section or do we reorder?

2.2: I got this wrong also. But are you sure?? I think it might be better to remove this, per my comments about 1.10 above.

I use real code for all these and the answer is what I implied as correct. I still am not 100% sure why we would cut in this case because I'm just asking for the output of the function essentially which uses the local variable.

4: consider splitting this into 2 code blocks, so they can more easily see the output of the provided code- not possible currently because of all the ___

Commented this out instead? Does that work?

I'm having a problem with docker. For ex 4, I press run code and get an error. Then I deleted all the ___ and pressed "run code" again but nothing happens! Does this happen to you as well??

Can you try it again now that I've commented things out? I can explain the issue at our meeting.

9.9: I'm not sure, pls check if the default part needs to be in more of a standardized format in the numpy style, e.g. "default = 2" or "default : 2"

This is now the format as recommended by the documentation

we should clarify that part of the use is people who are directly looking at the code, not just this ? type thing

? Je ne comprend pas

13.5: Argh, I know I keep contradicting myself, but I wonder if we should have just gone with if type(numerical_list) != list instead, so they didn't have to learn something new here. Let's discuss at the next meeting. ;;

DISCUSS

there's a confusion here between what "an exception" is. Is this an exception? Exception("You are not using a list for the numerical_list input.") I guess so. But in my head I immediately think of different types of exceptions if someone talks about different exceptions. So maybe we shouldn't muddy the waters by saying that Exception("A") and Exception("B") are two different exceptions.

This needs elaboration please

17.6: "It’s also good to do multiple test to for different list sizes" (multiple grammar issues). ;; we should be consistent here and use isinstance if that's what we're teaching

I initially included isinstance since it's the easiest way to test if an object is of type pd.DataFrame. Instead I removed it and added a hint in the assignment to use that function. Is this ok?

17.11: I'm a bit confused about this, because this code does not run. Would return list() be better? I would prefer something that runs.

What do you mean? which code?

Is this ordering taken from somewhere?

Your lecture actually 😂 actually. See here

3: have they heard the term pseudo-code before?

No but I explain it in this slide. Is that not enough?

18.1: put "not" in bold/italics like 18.2

Are you looking at the same question as me?

19: missing a space in the last choice;; also, I don't know, this one is a bit subjective. There might be a purpose to those tests. for bad tests, what if you add a docstring to the function that shows the inputs are supposed to be numbers, and then have a silly test like multiply(["a", 5]) == "aaaaa", which passes, but doesn't really test what the function is supposed to be doing?

Remove then?

21: do we really need the messages here?;; again, we should decide if we're doing type(x) == or isinstance. I wonder if maybe type(x) == is better. but my problem is an ego/vanity problem, that I don't want to look like I don't know. In such cases I usually pick the way that's better for students but add a little disclaimer like "See isinstance for a more advanced approach."

Adding messages just so they have familiarity with it and removed isinstance

Or could we go with the example from DSCI 511 instead?

I've been using DSCI 511 as a resource. Which example are you referring to?

22.9: needs a bit of work. let's be more specific about the pain points:

Let's talk about this a bit more.

1) argument to function with same variable ( uses the one with the argument)

2) remove 1.10

3) 1.16 show function -> call it on same slide when call 1.17 show unheated function, calling function and cereal all on same slide and function still worked (unheated cereal)

If you do have side effects you document them HERE is where we mention side effects (in docstring)

exercise 4) pass in weight_lb and remove return statement. change wording to just global and local environment

change "loading to blank"

sentence for reading document

isinstance add note in content-> there is a more robust way using isinstance (see here ->) we don't want to over complicate this In the interest of avoiding new syntax (this is a simple way of doing this, for a more robust approach see link)

17.15 return empty list here

peusdo code (say in the comments )

18.2 BOLD not

put question separately
1. 4 - 22.6

UBC-MDS / programming-in-python-for-data-science

Feedback Module 6 #45