Add _apply_list_op, each, map, filter to ComplexBase

[cifkao]

• Adding an _apply_list_op method that recursively applies a list operation to list attributes.
• Implementing remove_duplicate, remove_invalid and sort using this method (seems to fix remove_invalid() makes object even more invalid #49).
• Adding each, map and filter functions.

Example for filter:

def is_in_range(obj, a, b):
    if hasattr(obj, "time"):
        return obj.time >= a and obj.time <= b
    return True

# Crop to a given time range
music.filter(functools.partial(is_in_range, a=4, b=8)).adjust_time(lambda t: t - 4)

Example for each:

def fix_pitches(obj):
    if hasattr(obj, "pitch") and obj.pitch < 0:
        obj.pitch += (-obj.pitch + 11) // 12 * 12

# Transpose and fix invalid pitches
music.transpose(-60).each(fix_pitches)

[cifkao]

Related to #32.

[salu133445]

The functions map, filter and each can be quite handy, although they look pretty non-pythonic to me.

For map and each function, I think a more pythonic way is to have a for loop iterating over all the objects regardless of their types (similar to music21.score.flat). We could have a method Music.objects() that returns an iterator on all the objects. Then, we can do something as follows, which seems more pythonic to me.

for obj in music.objects():
    fix_pitches(obj)

(We cannot achieve the filter function as above though as it needs to modify the lists.)

[cifkao]

map is still useful, because it replaces values with new ones (maybe you can think of a name for it that better reflects this functionality). But replacing each with a flat iterator sounds nice. objects sounds quite generic, I think flat, like in numpy or music21, is better.

[salu133445]

Oops. You are right. map does not work that way. We could have a list iterator and do as follows.

for list_ in music.lists():
    list_[:] = map(func, list_)

Similarly, for filter, we could do the following.

for list_ in music.lists():
    list_[:] = [obj for obj in list_ if is_in_range(obj, a=4, b=8)]

Neither is that pythonic though.

[salu133445]

Also, isn't map more like apply in PyTorch? Python's built-in map returns a new iterable, but the one in this pull request does modify the lists in place.

[salu133445]

Music.objects() is probably not a good one as it sounds like everything, including non-list attributes, will be included. Music.flat() sounds good to me.

[salu133445]

It seems that Music._apply_list_op() is rather powerful. Why don't we make it Music.apply()? This could perhaps simplify map and filter by using apply with built-in map and filter.

[cifkao]

Also, isn't map more like apply in PyTorch? Python's built-in map returns a new iterable, but the one in this pull request does modify the lists in place.

Yes, a bit, but apply in PyTorch also doesn't replace the values. This map is like map but in-place, I'm not sure what's a good name for it. Maybe apply would work actually?

It seems that Music._apply_list_op() is rather powerful. Why don't we make it Music.apply()? This could perhaps simplify map and filter by using apply with built-in map and filter.

I was thinking about that. But the thing is that it accepts a function with 3 different parameters (of which I'm currently using only 2 actually...) and so (1) it can't be used with existing functions like built-in map and filter and (2) I thought it would be difficult to document it clearly.

[salu133445]

It does behave like a map on all the lists but does not return anything.

>>> a, b, c = [0], [1], [2]
>>> map(lambda x: x.append(-1), [a,b,c])
<map at 0x23c92c3bf88>
>>> print(a, b, c)  # has no effect as map returns an iterator
[0] [1] [2]
>>> list(map(lambda x: x.append(-1), [a,b,c]))
[None, None, None]
>>> print(a, b, c)
[0, -1] [1, -1] [2, -1]

[salu133445]

In fact, adjust_time is not really pythonic either. It would be better if we could iterate over all timestamped objects.

[salu133445]

I do think implementing remove_duplicate, remove_invalid and sort in this way is fine, and it seems to fix #49. For map, filter and each (and possibly apply), we could open another pull request and see if we can come up with a more consistent, pythonic way to implement them.

[cifkao]

In fact, adjust_time is not really pythonic either. It would be better if we could iterate over all timestamped objects.

If it weren't for the intricacies of adjusting durations...

[cifkao]

OK, I removed the methods.

I replaced _apply_list_op with _traverse_lists, which seems more pythonic. This also allowed me to implement flat.

[salu133445]

• The implementation of _traverse_lists looks good to me.
• I am not a fan of flat in music21. We should use a more obvious and pythonic name. For now, I can only come up with lists() and lists_with_names(). There should be better options.
• I already fixed remove_invalid() makes object even more invalid #49 by 8899576, but I am not sure which would be faster

[salu133445]

I think this might be problematic as it will keep reusing the generator created at __init__. We probably don't want this behavior. Also, I am not a fan of having flat as a property. Could we simply make it Music.lists(), which is rather straightforward from its name except that it's recursive?

[cifkao]

Right, I didn't realize that.

How about this? flat is now reusable, and it's always the same object, so it's not a problem to have it as a property.

If you still insist on it being a function, why not call it e.g. walk_lists?

[salu133445]

I just want it to behave more like dict.items() or dict.values() rather than numpy.ndarray.flat or music21.stream.Stream.flat. It's quite common in Python to have a function that returns an iterator.

[cifkao]

True. Is walk_lists() OK then? The name should give a hint that the return value is good only for (one) iteration.

BTW dict.items() and dict.values() also return objects that support more than just iteration (e.g. len()).

[cifkao]

Actually, lists() suggests that it would iterate over lists. But my flat iterates over list items, not lists themselves. So we need a better name. I think flat() (just changing the property to a function) or walk_lists() is OK.

[cifkao]

I made it list_items() and added list() to iterate over the actual lists.

[salu133445]

A return signature of either the value (like dict.values) or a tuple of the key and the value(attr, value) (like dict.items) would be more straightforward. Maybe we can make them two separate methods.

But if we do this, we can no longer make remove_invalid, remove_duplicate and sort work using this, right?

Or we could keep this unchanged and have two public wrappers lists and lists_with_names.

[cifkao]

Yes, I would definitely keep this and internal function.

[salu133445]

Type checking can be simplified if we change the return signature into Iterator[Tuple[str, List]].

[cifkao]

I think there must be a bug in mypy, it was complaining about things whose type is actually known...

[cifkao]

OK, I replaced flat with list_items() and also added lists().

Example:

>>> list(m.lists())
[('tempos', []),
 ('key_signatures', []),
 ('time_signatures', []),
 ('downbeats', []),
 ('lyrics', []),
 ('annotations', []),
 ('tracks',
  [Track(program=33, is_drum=False, name='BB Bass', notes=[Note(time=0, pitch=34, duration=12, velocity=59), Note(time=18, pitch=43, duration=17, velocity=74), Note(time=36, pitch=39, duration=5, velocity=62), ...])])]

>>> list(m.list_items())
[Track(program=33, is_drum=False, name='BB Bass', notes=[Note(time=0, pitch=34, duration=12, velocity=59), Note(time=18, pitch=43, duration=17, velocity=74), Note(time=36, pitch=39, duration=5, velocity=62), ...])]

>>> list(m.lists(recursive=True))
[('tempos', []),                                                                                                                                                                                            
 ('key_signatures', []),                                                                                                                                                                                    
 ('time_signatures', []),                                                                                                                                                                                   
 ('downbeats', []),                                                                                                                                                                                         
 ('lyrics', []),                                                                                                                                                                                            
 ('annotations', []),                                                                                                                                                                                       
 ('notes',                                                                                                                                                                                                  
  [Note(time=0, pitch=34, duration=12, velocity=59),                                                                                                                                                        
   Note(time=18, pitch=43, duration=17, velocity=74),                                                                                                                                                       
   Note(time=36, pitch=39, duration=5, velocity=62),                                                                                                                                                        
   ...]),
 ('chords', []),
 ('lyrics', []),
 ('annotations', []),
 ('tracks',
  [Track(program=33, is_drum=False, name='BB Bass', notes=[Note(time=0, pitch=34, duration=12, velocity=59), Note(time=18, pitch=43, duration=17, velocity=74), Note(time=36, pitch=39, duration=5, velocity=62), ...])])]

>>> list(m.list_items(recursive=True))
[Note(time=0, pitch=34, duration=12, velocity=59),                                                                                                                                                          
 Note(time=18, pitch=43, duration=17, velocity=74),                                                                                                                                                         
 Note(time=36, pitch=39, duration=5, velocity=62),
 ...,
 Track(program=33, is_drum=False, name='BB Bass', notes=[Note(time=0, pitch=34, duration=12, velocity=59), Note(time=18, pitch=43, duration=17, velocity=74), Note(time=36, pitch=39, duration=5, velocity=62), ...])]