is this pythonic?

Discussion in 'Python' started by TP, Jan 21, 2009.

  1. TP

    TP Guest

    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)
     
    TP, Jan 21, 2009
    #1
    1. Advertising

  2. On Wed, 21 Jan 2009 16:16:32 +0100, TP wrote:
    >
    > 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.)

    --
    To email me, substitute nowhere->spamcop, invalid->net.
     
    Peter Pearson, Jan 21, 2009
    #2
    1. Advertising

  3. TP

    alex23 Guest

    On Jan 22, 1:16 am, TP <> wrote:
    > 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.
     
    alex23, Jan 21, 2009
    #3
  4. TP

    MRAB Guest

    alex23 wrote:
    > On Jan 22, 1:16 am, TP <> wrote:
    >> 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]
     
    MRAB, Jan 21, 2009
    #4
  5. TP

    TP Guest

    alex23 wrote:

    > 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)
     
    TP, Jan 21, 2009
    #5
  6. TP

    alex23 Guest

    On Jan 22, 3:27 am, MRAB <> wrote:
    > 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.
     
    alex23, Jan 21, 2009
    #6
  7. TP

    MRAB Guest

    alex23 wrote:
    > On Jan 22, 3:27 am, MRAB <> wrote:
    >> 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):
    >

    [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)'.
     
    MRAB, Jan 21, 2009
    #7
  8. TP

    alex23 Guest

    On Jan 22, 3:34 am, TP <> wrote:
    > >     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'}}

    > > 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?


    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
     
    alex23, Jan 21, 2009
    #8
  9. TP

    alex23 Guest

    On Jan 22, 3:56 am, MRAB <> wrote:
    > 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.
     
    alex23, Jan 21, 2009
    #9
  10. TP

    Guest

    On Jan 21, 12:34 pm, TP <> wrote:
    > alex23 wrote:
    > > 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:

    >>> o=[]
    >>> i=0
    >>> ln=len(l)
    >>> while i<ln:

    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:

    >>> for index, record in enumerate(l):

    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.
     
    , Jan 21, 2009
    #10
  11. TP

    Peter Otten Guest

    TP wrote:

    > 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'}}
    >>> del lookup["ti"]
    >>> lookup

    {'to': {'value': 2, 'title': 'to'}}

    If you later have to accomodate for multiple dictionaries with the same
    title use lists of dictionaries as values:

    >> from collections import defaultdict
    >>> lookup = defaultdict(list)
    >>> for item in items:

    .... lookup[item["title"]].append(item)
    ....
    >>> lookup

    defaultdict(<type 'list'>, {'to': [{'value': 2, 'title': 'to'}], 'ti':
    [{'value': 'coucou', 'title': 'ti'}]})
    >>> del lookup["ti"]
    >>> lookup

    defaultdict(<type 'list'>, {'to': [{'value': 2, 'title': 'to'}]})

    Peter
     
    Peter Otten, Jan 22, 2009
    #11
  12. TP

    TP Guest

    Peter Otten wrote:

    > If you can change the rest of your program to work smoothly with a
    > dictionary I would suggest the following:

    [snip]
    >>> from collections import defaultdict

    [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)
     
    TP, Jan 22, 2009
    #12
  13. TP

    Guest

    On Jan 21, 4:23 pm, Scott David Daniels <> wrote:
    > wrote:
    > > ... 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
    >


    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'])
     
    , Jan 22, 2009
    #13
  14. TP

    Guest

    On Jan 21, 4:23 pm, Scott David Daniels <> wrote:
    > wrote:
    > > ... 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
    >


    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']
     
    , Jan 22, 2009
    #14
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. kjockey

    pythonic malloc

    kjockey, Jul 21, 2003, in forum: Python
    Replies:
    7
    Views:
    294
    Brad Hards
    Jul 23, 2003
  2. Tom Evans
    Replies:
    9
    Views:
    311
    Tom Evans
    Oct 29, 2003
  3. zapazap
    Replies:
    2
    Views:
    392
    Mark Hammond
    Jan 26, 2004
  4. zapazap
    Replies:
    2
    Views:
    412
    zapazap
    Mar 2, 2004
  5. Carl J. Van Arsdall
    Replies:
    4
    Views:
    502
    Bruno Desthuilliers
    Feb 7, 2006
Loading...

Share This Page