1
\$\begingroup\$

I would like to do the following:
I have a list data of 3-tuples that I need to go through, for each tuple base_tuple I want to know the index index_max of the maximum value held in base_tuple and replace base_tuple with a new 3-tuple new_tuple that only holds 0s except for the value at index index_max which is set at a fixed value fixed_value.
For instance, with a fixed_value of 127 and a base_tuple of (12, 55, 27), I expect new_tuple to be (0, 127, 0).

This is the code I use:

fixed_value = 255
# data is the list of tuples
for i, base_tuple in enumerate(data):
    index_max = base_tuple.index(max(base_tuple))
    new_tuple = tuple(fixed_value*(i==index_max) for i in range(3))
    data[i] = new_tuple

It works but it looks more like a hack than proper python... Any idea how to do this in a more pythonic way?

\$\endgroup\$
1

1 Answer 1

2
\$\begingroup\$

Code organisation

At this moment, the code is there with no context. In order to make it easier to unerstand, reuse and test, you could try to define in a a function on its own.

It could for instance:

  • take data as a parameter
  • take fixed_value as a parameter (with a default of 255 ?)

In-place changes

When trying to explain or test the function, things are make slighly harder by the fact that the input is modified. In your case, it may be just as easy to return a new list of values.

Here is a tweet from Raymond Hettinger (Python core developer and trainer):

Today's super-easy #python student question.

Q. What are the best practices for modifying a list while looping over it?

A. Don't. Seriously, just make a new list and avoid hard-to-read code with hard-to-find bugs.

At this stage, we have something like:

new_data = []
for i, base_tuple in enumerate(data):
    index_max = base_tuple.index(max(base_tuple))
    new_tuple = tuple(fixed_value*(i==index_max) for i in range(3))
    new_data.append(new_tuple)
return new_data

But the call to enumerate is not required anymore and we can write:

new_data = []
for base_tuple in data:
    index_max = base_tuple.index(max(base_tuple))
    new_tuple = tuple(fixed_value*(i==index_max) for i in range(3))
    new_data.append(new_tuple)
return new_data

More functions

We could extract out the tuple computation in a function on its own. We would have something like:

def get_new_tuple(base_tuple, fixed_value):
    index_max = base_tuple.index(max(base_tuple))
    return tuple(fixed_value*(i==index_max) for i in range(3))

def get_new_val(data, fixed_value = 255):
    # data is the list of tuples
    new_data = []
    for base_tuple in data:
        new_data.append(get_new_tuple(base_tuple, fixed_value))
    return new_data

(Documentation and function names should be improved).

Now, this is a good occasion to use list comprehension in get_new_val.

At this stage, renaming the function, the parameter and adding a simple test, we have:

def get_new_tuple(base_tuple, fixed_value):
    index_max = base_tuple.index(max(base_tuple))
    return tuple(fixed_value*(i==index_max) for i in range(3))

def get_new_tuples(tuples, fixed_value = 255):
    return [get_new_tuple(tup, fixed_value) for tup in tuples]

def test_get_new_tuples():
    # This could/should be written with a proper unit-test framework
    data = [(12, 55, 27), (260, 55, 27), (12, 55, 255)]
    output = get_new_tuples(data)
    assert output == [(0, 255, 0), (255, 0, 0), (0, 0, 255)]

Improving get_new_tuple

There is the magic number 3. It would make sense to be more generic and accept tuples of any length.

For instance:

def get_new_tuple(base_tuple, fixed_value):
    index_max = base_tuple.index(max(base_tuple))
    return tuple(fixed_value*(i==index_max) for i in range(len(base_tuple)))

And we can test it with more unit tests:

def test_get_new_tuple():
    tup = (12, 55, 27)
    output = get_new_tuple(tup, 255)
    assert output == (0, 255, 0)
    tup = (12, 55, 27, 42)
    output = get_new_tuple(tup, 255)
    assert output == (0, 255, 0, 0)

Also, the pattern range(len(foo)) usually suggests we could do things in a better way. For instance, we could use enumerate:

return tuple(fixed_value*(i==index_max) for i, _ in enumerate(base_tuple))

Edge case

Another question we could ask ourselves is about the behavior of the function where the max value appears more than once. It probably deserves to be documented and properly tested. Based on what we want, we may have to change the implementation.

Another edge case that could be tested/documented is how the empty tuple is handled.

\$\endgroup\$
2
  • \$\begingroup\$ thank you for your feedback. I actually was more concerned about the way new_tuple is created (using kind of a 'logic mixed with arithmetic' hack), don't you think it looks like a poor way to do what's intended? Also, should I edit my question so that the code meets your Code organisation piece of advice? \$\endgroup\$
    – Louisono
    Apr 8, 2019 at 10:50
  • \$\begingroup\$ @Louisono: Do not edit your question after you have got an answer, especially if that edit would invalidate the answer. Include it in your code and ask a follow-up question if the changes are significant enough. \$\endgroup\$
    – AlexV
    Apr 8, 2019 at 11:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.