project1-feedback

[weilu]

dec-python-course/1-foundations/project-1-schelling-model/project1-solutions.py

Line 21 in 8248dd7

N = len(grid)

In python variables named with all caps are constants. Since the dimension of the city changes with different input perhaps it's not most appropriate to define it as a constant?

[kbjarkefur]

dec-python-course/1-foundations/project-1-schelling-model/project1-solutions.py

Line 21 in 8248dd7

N = len(grid)

In python variables named with all caps are constants. Since the dimension of the city changes with different input perhaps it's not most appropriate to define it as a constant?

I noted that as well. I think we should update for later, but no need to do before session, as in this exact example grid size is not changing. But since it is a better practice I think we should change it for the future. And the name can be city_size instead of n as N/n should is rarely used for a dimension of a matrix, more so the total number of items in a matrix.

[kbjarkefur]

Make sure test_functions passes same type as the doc string says. I.e. lists and not tuples.

[weilu]

Consider putting the prep cell code in a single cell – easier to run

[weilu]

The enumerate example, consider changing the variable names to i, val as j is conventionally used as a name for index var

[weilu]

Tests for similarity function may not be sufficient. One participant's solution for similarity passed all the tests but failed the tests for hypothetical similarity. Turned out his solution for similarity was not fully correct

[kbjarkefur]

tell people to read the doc string properly to understand what is inputted and what is expected to be outputted

[weilu]

Feedback from a participant: probably good to start with an easier task, like finding blank/available slot in a city, with a small grid e.g. 2x2 to help ease the participants in

[weilu]

One participant emailed me asking about why some cells are excluded from the list_available's expectations/tests. Turned out they didn't realize for a cell to be available it has to be unoccupied (they were just checking it's above threshold). Perhaps highlight in the instruction about both conditions?