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 0
s 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?
-
\$\begingroup\$ stackoverflow.com/questions/2474015/… \$\endgroup\$aghast– aghast2019年04月08日 13:04:11 +00:00Commented Apr 8, 2019 at 13:04
1 Answer 1
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.
-
\$\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– Louisono2019年04月08日 10:50:23 +00:00Commented 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– AlexV2019年04月08日 11:28:55 +00:00Commented Apr 8, 2019 at 11:28