Linted

[evaherrada]

Oh, meant to reply that to your review

[KeithTheEE]

I don't think the reply went through?

[evaherrada]

@KeithTheEE Oh, yeah, I see. I started a review but didn't finish it.

@KeithTheEE So the reason I'm doing that is that in this case it's editing the values and you can't do that if you use enumerate or for i in object, in the list, but below, it is not. Or at least that's my understanding of it, but if I'm wrong that would be good to know

[KeithTheEE]

That makes sense. I think you're right in how it behaves. I was just curious if that was the reason between the two styles, or if there was a deeper reason I wasn't aware of. Give me a minute to duplicate this to test the behavior and be sure, but it looks good to me otherwise.

Thank you for clarifying the reasoning!

[KeithTheEE]

Ok I dug into it a bit more and I think I've got a clearer understanding of what's going on. I think enumerate should work here because we're only changing attributes of the pin.

So the code should look like, for row, row_pin in enumerate(self.row_pins):

This has the added benefit of changing the following lines from self.row_pins[row].direction = Direction.OUTPUT, to row_pin.direction = Direction.OUTPUT, etc, which should make the code more readable as an added bonus.

Below is the sample code snippets I wrote to make sure it's possible, and a walk through for what each snippet is doing as best I understand it. Hopefully if I messed something up, these snippets will make it easier to correct me. I also tested it in the REPL to be sure it performs as expected in circuit python as well.

If the list values are just simple objects like ints,

rows = [x+5 for x in range(4)]
print(rows)
for row_index, row_value in enumerate(rows):
    rows[row_index] = row_value-2
print(rows)

Gives

>>> [5, 6, 7, 8]
>>> [3, 4, 5, 6]

However if we do this,

rows = [x+5 for x in range(4)]
print(rows)
for row_index, row_value in enumerate(rows):
    row_value = row_value-2
print(rows)

We end up with

>>> [5, 6, 7, 8]
>>> [5, 6, 7, 8]

Because rows never gets the adjusted value put back into it. We just evaluate the row_value - 2, and assign that to row_value. At no point does the list's content get updated.

Now with a class, it's a bit different because the list isn't filled with copies of the class, but pointers to where the object is stored in memory--this way it saves on memory costs. So when we adjust the value of an attribute in the class, we don't need to worry about putting the adjusted class object back into the list.

I'm going to make a fake pin class so it looks similar to the pin class we'd have in the board, and run through an abridged version of the row value code using the enumerate option:

class Fake_Pin(object):
    def __init__(self):
        self.val = False
        self.direction = "INPUT"
        self.pull = "UP"

row_pins = [Fake_Pin() for x in range(4)]

print("Before Adjusting Values")
for pin in row_pins:
    print(pin.val)

for row_index, row_pin_value in enumerate(row_pins):
    row_pin_value.val = True

print("\nAfter Adjusting Values")
for pin in row_pins:
    print(pin.val)

After running that, we end up with:

Before Adjusting Values
False
False
False
False

After Adjusting Values
True
True
True
True

While we never put anything back into the list, we do adjust the value of an attribute of an item in the list. The list is still filled with pointers to where each class is in memory, so the change in the attributes is reflected when we look at it later on.

[kattni]

I'm going to have Dan test it when this is ready - he has the hardware. Please tag me again when this is ready for testing, and I'll bring Dan in at that time (so he doesn't have to pay attention for updates).

[evaherrada]

@KeithTheEE Oh, you are completely right. Thanks for the writeup. That definitely helped me understand it better.

[KeithTheEE]

@dherrada I'm glad to hear that! It threw me for such a loop I had to sit down and write tests for a while. I'm glad it helped you too!

[kattni]

@dhalbert Hello! Please test this update with the matrixkeypad_4x4 example. Thank you!

[dhalbert]

I tested the 4x4 example with a 3x4 keypad (I don't have a 4x4). It worked fine.