is this pythonic?

T

TP

Hi,

Is the following code pythonic:
l=[{"title":"to", "value":2},{"title":"ti","value":"coucou"}]
dict = [ dict for dict in l if dict['title']=='ti']
l.remove(*dict)
l
[{'title': 'to', 'value': 2}]

Precision: I have stored data in the list of dictionaries l, because in my
application I am sure that "title" is unique for each record. But perhaps
it is better to imagine that someday it will not be anymore the case? And
rather use a data storage as the following?

l = { '001':{"title":"to", "value":2}, '002'
{"title":"ti","value":"coucou"}}

The problem with this storage is that it implies to manipulate some "ids"
that have not any meaning for a humain being (001, 002, etc).

Thanks a lot for you opinion,

--
python -c "print ''.join([chr(154 - ord(c)) for c in '*9(9&(18%.\
9&1+,\'Z4(55l4('])"

"When a distinguished but elderly scientist states that something is
possible, he is almost certainly right. When he states that something is
impossible, he is very probably wrong." (first law of AC Clarke)
 
P

Peter Pearson

Is the following code pythonic:
l=[{"title":"to", "value":2},{"title":"ti","value":"coucou"}]
dict = [ dict for dict in l if dict['title']=='ti']
l.remove(*dict)
l
[{'title': 'to', 'value': 2}]

Precision: I have stored data in the list of dictionaries l, because in my
application I am sure that "title" is unique for each record. But perhaps
it is better to imagine that someday it will not be anymore the case?
[snip]

1. You probably don't want to use the name "dict".

2. I believe this code will fail if the number of dictionaries
with title="ti" is not exactly 1. It that your intention?
(You probably answered this question in the last paragraph
quoted above, but I can't make it out.)
 
A

alex23

Is the following code pythonic:
l=[{"title":"to", "value":2},{"title":"ti","value":"coucou"}]
dict = [ dict for dict in l if dict['title']=='ti']
l.remove(*dict)
l
[{'title': 'to', 'value': 2}]

Try not to use 'dict' or the name of any of the other built-in types
as labels.

You're stepping through an entire list just to pass another list to
l.remove to step through and remove items from...in fact, given that
list.remove deletes the -first- occurance of the item, you're asking
it to loop through -again- to find the matching element which you've -
already- detected. A better and cleaner approach would be to step
through the list -once- and remove the item when you find it:

for index, record in enumerate(l):
if record['title'] == 'ti':
l.pop(index)

Or you could just use a list comprehension:

l = [d for d in l if d['title'] == 'ti']
Precision: I have stored data in the list of dictionaries l, because in my
application I am sure that "title" is unique for each record. But perhaps
it is better to imagine that someday it will not be anymore the case?

It's always better to design for what you know you need, not what you
may possibly need in the future. You say that you are sure that record
titles are unique, so why not use them as the dictionary keys, with
the values as the values:

records = {'ti': 1, 'to': 2}

This way your code can be replaced with:

value = records.pop('ti') # if you want to know the value
del records['ti'] # if you just want to delete the entry

It's a lot simpler to work with and extend.
 
M

MRAB

alex23 said:
Is the following code pythonic:
l=[{"title":"to", "value":2},{"title":"ti","value":"coucou"}]
dict = [ dict for dict in l if dict['title']=='ti']
l.remove(*dict)
l
[{'title': 'to', 'value': 2}]

Try not to use 'dict' or the name of any of the other built-in types
as labels.

You're stepping through an entire list just to pass another list to
l.remove to step through and remove items from...in fact, given that
list.remove deletes the -first- occurance of the item, you're asking
it to loop through -again- to find the matching element which you've -
already- detected. A better and cleaner approach would be to step
through the list -once- and remove the item when you find it:

for index, record in enumerate(l):
if record['title'] == 'ti':
l.pop(index)
[snip]
FYI, you shouldn't modify a list you're iterating over.
Or you could just use a list comprehension:

l = [d for d in l if d['title'] == 'ti']
The for-loop was removing the item where there's a match, so the list
comprehension in this case should keep the item where there _isn't_ a match:

l = [d for d in l if d['title'] != 'ti']

[remainder snipped]
 
T

TP

alex23 said:
Try not to use 'dict' or the name of any of the other built-in types
as labels.

Ooooops... Moreover I know it...
You're stepping through an entire list just to pass another list to
l.remove to step through and remove items from...in fact, given that
list.remove deletes the -first- occurance of the item, you're asking
it to loop through -again- to find the matching element which you've -
already- detected. A better and cleaner approach would be to step
through the list -once- and remove the item when you find it:

for index, record in enumerate(l):
if record['title'] == 'ti':
l.pop(index)

Ok, I will use this solution. But it is less pythonic than list
comprehensions.
Or you could just use a list comprehension:

l = [d for d in l if d['title'] == 'ti']

Perhaps you mean rather:

l = [d for d in l if d['title'] != 'ti']
?

In fact, I cannot use this solution, because I want to get back the
dictionary with title 'ti', for another use (in fact, to add it to another
list, see below).
Precision: I have stored data in the list of dictionaries l, because in
my application I am sure that "title" is unique for each record. But
perhaps it is better to imagine that someday it will not be anymore the
case?

It's always better to design for what you know you need, not what you
may possibly need in the future. You say that you are sure that record
titles are unique, so why not use them as the dictionary keys, with
the values as the values:

records = {'ti': 1, 'to': 2}

This way your code can be replaced with:

value = records.pop('ti') # if you want to know the value
del records['ti'] # if you just want to delete the entry

It's a lot simpler to work with and extend.

In fact, in my case, in cannot use this simple solution, because there are
other fields in each dictionary, not only 2. I was not clear in my post.
So my list is rather:
l=[{"title":"to", "color":"blue", "value":2}
{"title":"ti", "color":"red", "value":"coucou"}]

So, I will rather use your solution:

for index, record in enumerate(l):
if record['title'] == 'ti':
to_add_in_another_list = l.pop(index)
another_list.append(to_add_in_another_list )
It's always better to design for what you know you need, not what you
may possibly need in the future.

Ok. Do all the programmers agree with this principle?

--
python -c "print ''.join([chr(154 - ord(c)) for c in '*9(9&(18%.\
9&1+,\'Z4(55l4('])"

"When a distinguished but elderly scientist states that something is
possible, he is almost certainly right. When he states that something is
impossible, he is very probably wrong." (first law of AC Clarke)
 
A

alex23

FYI, you shouldn't modify a list you're iterating over.

But I'm not. I'm building a new list and binding it to the same name
as the original, which works perfectly fine (unless I'm missing
something):
l = range(100)
l = [d for d in l if not d % 5]
l
[0, 5, 10, 15, 20, 25, 30, 35, 40, 45, 50, 55, 60, 65, 70, 75, 80, 85,
90, 95]
The for-loop was removing the item where there's a match, so the list
comprehension in this case should keep the item where there _isn't_ a match:

l = [d for d in l if d['title'] != 'ti']

Now that's a mistake. Cheers for the catch.
 
M

MRAB

alex23 said:
But I'm not. I'm building a new list and binding it to the same name
as the original, which works perfectly fine (unless I'm missing
something):
[snip]
I was referring to the code:

for index, record in enumerate(l):
if record['title'] == 'ti':
l.pop(index)

where you are enumerating and iterating over 'l', but also modifying 'l'
with 'l.pop(index)'.
 
A

alex23

    for index, record in enumerate(l):
        if record['title'] == 'ti':
            l.pop(index)

Ok, I will use this solution. But it is less pythonic than list
comprehensions.

Are you asking if it's less pythonic, or asserting? Because you'll
find a lot of disagreement here: list comprehensions are used for
constructing lists, not manipulating them.
Perhaps you mean rather:

l = [d for d in l if d['title'] != 'ti']
?

You are correct. I should never post past 3am :)
In fact, in my case, in cannot use this simple solution, because there are
other fields in each dictionary, not only 2. I was not clear in my post.
So my list is rather:
l=[{"title":"to", "color":"blue", "value":2}
{"title":"ti", "color":"red", "value":"coucou"}]

I still find this a lot simpler:

records = {'to': {'color': 'blue', 'value': '2'}, 'ti': {'color':
'red', 'value': 'coucou'}}
Ok. Do all the programmers agree with this principle?

Have you seen the size of some of the threads here? It's hard to get
two programmers to agree on variable names... :) But it's a practice
that has served me well and it even has a catchy name[1]. Getting what
you -need- to work is effort enough, if you don't have a definite use
case for a feature then how do you know you've implemented it
correctly? Write it when you need it.

1: http://en.wikipedia.org/wiki/You_Ain't_Gonna_Need_It
 
A

alex23

I was referring to the code:

     for index, record in enumerate(l):
         if record['title'] == 'ti':
             l.pop(index)

where you are enumerating and iterating over 'l', but also modifying 'l'
with 'l.pop(index)'.

Ack, you're absolutely correct.

TP: my mistake, the for loop won't work at all. I'd really recommend
using the dictionary-based solution.

Apologies all around.
 
P

pruebauno

alex23 said:
Try not to use 'dict' or the name of any of the other built-in types

So my list is rather:
l=[{"title":"to", "color":"blue", "value":2}
{"title":"ti", "color":"red", "value":"coucou"}]

So, I will rather use your solution:

for index, record in enumerate(l):
    if record['title'] == 'ti':
        to_add_in_another_list = l.pop(index)
another_list.append(to_add_in_another_list )
If you have duplicates this will not work. You will have to do
something like this instead:
if l['title']=='ti':
o.append(l.pop(i))
ln-=1
else:
i+=1



If you don't have duplicates you can extract the one and exit early:
if record['title'] == 'ti':
to_add_in_another_list = l.pop(index)
break

I don't know if these are more pythonic, they should be more efficient
for longer lists though.
 
P

Peter Otten

TP said:
Hi,

Is the following code pythonic:
l=[{"title":"to", "value":2},{"title":"ti","value":"coucou"}]
dict = [ dict for dict in l if dict['title']=='ti']
l.remove(*dict)
l
[{'title': 'to', 'value': 2}]

Precision: I have stored data in the list of dictionaries l, because in my
application I am sure that "title" is unique for each record. But perhaps
it is better to imagine that someday it will not be anymore the case? And
rather use a data storage as the following?

l = { '001':{"title":"to", "value":2}, '002'
{"title":"ti","value":"coucou"}}

The problem with this storage is that it implies to manipulate some "ids"
that have not any meaning for a humain being (001, 002, etc).

Thanks a lot for you opinion,

If you can change the rest of your program to work smoothly with a
dictionary I would suggest the following:
items = [{"title":"to", "value":2},{"title":"ti","value":"coucou"}]
lookup = dict((item["title"], item) for item in items)
lookup
{'to': {'value': 2, 'title': 'to'}, 'ti':
{'value': 'coucou', 'title': 'ti'}}
{'to': {'value': 2, 'title': 'to'}}

If you later have to accomodate for multiple dictionaries with the same
title use lists of dictionaries as values:
.... lookup[item["title"]].append(item)
....
defaultdict(<type 'list'>, {'to': [{'value': 2, 'title': 'to'}]})

Peter
 
T

TP

Peter said:
If you can change the rest of your program to work smoothly with a
dictionary I would suggest the following: [snip]
[snip]

Thanks a lot.
I didn't know defaultdict. It is powerful.

I begin to understand that people prefer using dictionaries than lists, so
as to take advantage of their automatic lookup feature.

--
python -c "print ''.join([chr(154 - ord(c)) for c in '*9(9&(18%.\
9&1+,\'Z4(55l4('])"

"When a distinguished but elderly scientist states that something is
possible, he is almost certainly right. When he states that something is
impossible, he is very probably wrong." (first law of AC Clarke)
 
P

pruebauno

... If you have duplicates this will not work. You will have to do
something like this instead:
o=[]
i=0
ln=len(l)
while i<ln:
   if l['title']=='ti':
           o.append(l.pop(i))
           ln-=1
   else:
           i+=1


Or the following:
     indices = [i for i,d in enumerate(l) if d['title']=='ti']
     for i in reversed(indices): # so del doesn't affect later positions
         del l

--Scott David Daniels
(e-mail address removed)


Cool. How come I didn't think of that! That means I can create an evil
one liner now >:).

replacecount=len([o.append(l.pop(i)) for i in reversed(xrange(len(l)))
if l['title']=='ti'])
 
P

pruebauno

... If you have duplicates this will not work. You will have to do
something like this instead:
o=[]
i=0
ln=len(l)
while i<ln:
   if l['title']=='ti':
           o.append(l.pop(i))
           ln-=1
   else:
           i+=1


Or the following:
     indices = [i for i,d in enumerate(l) if d['title']=='ti']
     for i in reversed(indices): # so del doesn't affect later positions
         del l

--Scott David Daniels
(e-mail address removed)


Cool. How come I didn't think of that!. Now I can write my evil one
liner >:).

o=[l.pop(i) for i in reversed(xrange(len(l))) if l['title']=='ti']
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,904
Latest member
HealthyVisionsCBDPrice

Latest Threads

Top