Allow iterables of qualified class names in Stream.__getitem__ searches

[jacobtylerwalls]

• Since v7, square brackets on Streams are syntactic sugar for .recurse().getElementsByClass(), but unlike that long form, the short form didn't cope with iterables of qualified class names such as [note.Note, note.Rest]:

TypeError: Streams can get items by int, slice, class, or string query; got <class 'tuple'>

Inspired by mailing list message.

Also,

• Remove the constraint that the types provided must be subclasses of Music21Object to match longhand syntax (Fixes Stream.__getitem__ is stricter about subclassing Music21Object than ClassFilter #1360)

[coveralls]

Coverage decreased (-0.005%) to 93.005% when pulling 6155e9d on gebc-multiple-classes into ee04424 on master.

[mscuthbert]

Looks good. One question and some doc updates.

[mscuthbert]

does this properly type the output for mypy? (just want to be sure that that was checked)

[jacobtylerwalls]

I think I'm not quite following -- can you explain what you mean? (mypy passes, and I didn't touch the return type)

[mscuthbert]

What I mean is something like, say you start typing:

for el in s[note.Note, chord.Chord]:
    print(el.pitches)
    print(el.recurse())

will the el.pitches line start to auto-suggest .pitches when you've typed .pi... (like does it know that all possible members have .pitches in it), and will mypy choke on the el.recurse() -- knowing that the union of note.Note and chord.Chord does not have .recurse() defined on all members -- or even one member.

And if you do:

for el in s[note.Note, chord.Chord]:
    if not isinstance(el, note.Note):
         print(el.closedPosition())

and know that this is fine? I'm just wondering if the typing is inferred from the ChangedM21Type.

And most importantly, does this still raise a mypy error:

for el in s[note.Note]:
     print(el.isMajorTriad())

while this passes?

for el in s[note.Note]:
     print(el.pitch)

I'll just grab the PR and play with it. But I'm wondering if putting the two different types of things in the same @overload might cause problems.

[mscuthbert]

better to demo these two things separately. If a class is given, then a RecursiveIterator of elements matching the class is returned, similar...

then later, "Multiple classes can be passed in separated by commas. Any element matching any of the classes will be returned." with the demo of that.

[mscuthbert]

not such a fan of this type of generator predicate. Clearer to make two separate elif statements, one for the single class and second one for the isIterable case. Students read this code and emulate it.

[mscuthbert]

Looks good -- I'll play with this and make the tiny quote/space change myself.

[mscuthbert]

What I mean is something like, say you start typing:

for el in s[note.Note, chord.Chord]:
    print(el.pitches)
    print(el.recurse())

will the el.pitches line start to auto-suggest .pitches when you've typed .pi... (like does it know that all possible members have .pitches in it), and will mypy choke on the el.recurse() -- knowing that the union of note.Note and chord.Chord does not have .recurse() defined on all members -- or even one member.

And if you do:

for el in s[note.Note, chord.Chord]:
    if not isinstance(el, note.Note):
         print(el.closedPosition())

and know that this is fine? I'm just wondering if the typing is inferred from the ChangedM21Type.

And most importantly, does this still raise a mypy error:

for el in s[note.Note]:
     print(el.isMajorTriad())

while this passes?

for el in s[note.Note]:
     print(el.pitch)

I'll just grab the PR and play with it. But I'm wondering if putting the two different types of things in the same @overload might cause problems.

[mscuthbert]

quotes and spacing. (Sorry that the new linter doesn't do doctests.

[mscuthbert]

No, in fact the new typing doesn't even know that el is a Music21Object. So the overload needs to be written separately.

There was actually a reason for the stricter [RecurseType] needing to be a Music21Object -- that's how the operator knew that everything coming out was still a Music21Object. There's a way to make it still work.

[mscuthbert]

For typing I think we're going to need to leave in the stricter version and probably make .getElementsByClass() stricter too. People will need to do the .getElementsByClass('RepeatMark') way, or .addFilter(lambda x: isinstance(x, RepeatMark)) ... or we'll make RepeatMark a Music21Object. That's probably the best way to do things.

Adding general t.Type as one of the things that could filter meant that we were losing the amazing type-inference that was one of the main points of doing this refactor. Unfortunately Python does not yet have a t.Type[t.Not[M21ObjType]]