Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor abjad.select() keywords #1464

Open
martin-ribot opened this issue Jun 4, 2022 · 3 comments
Open

Refactor abjad.select() keywords #1464

martin-ribot opened this issue Jun 4, 2022 · 3 comments
Assignees
Labels
Milestone

Comments

@martin-ribot
Copy link

Right now there are two equivalent but contradicting properties regarding logical ties. You can iterate a logical tie and inspect if it's trivial by using the is_trivial property. If it returns True, the logical tie is trivial, if it returns False it's not. However, when you select a logical tie, you can also specify in your selection the same property that needs opposing values to work. The property nontrivial, when set to True, makes is_trivial return False, when it's False it makes is_trivial return True.

I have the impression that the syntax
abjad.select.logical_ties(object, is_trivial=False)
is less confusing: you don't have to remember about nontrivial and is_trivial while producing the same result.

@trevorbaca
Copy link
Member

Yes, completely agree. This is part of a broader pattern that should change.

Several of the keywords used by the abjad.select.*() functions are ternary-valued parameters that, unhelpfully, masquerade as boolean-valued parameters. This is true of the grace=None, nontrivial=None, pitched=None keywords, and probably one or two others. The behavior is clear with examples.

Here's a voice with four logical ties:

voice = abjad.Voice("c'8 ~ c'8 d'8 ~ d'8 e'4 r4")
for logical_tie in abjad.select.logical_ties(voice):
    print(logical_tie)
LogicalTie(items=[Note("c'8"), Note("c'8")])
LogicalTie(items=[Note("d'8"), Note("d'8")])
LogicalTie(items=[Note("e'4")])
LogicalTie(items=[Rest('r4')])

Here's nontrivial=True:

for logical_tie in abjad.select.logical_ties(voice, nontrivial=True):
    print(logical_tie)
LogicalTie(items=[Note("c'8"), Note("c'8")])
LogicalTie(items=[Note("d'8"), Note("d'8")])

And here's nontrivial=False:

for logical_tie in abjad.select.logical_ties(voice, nontrivial=False):
    print(logical_tie)
LogicalTie(items=[Note("e'4")])
LogicalTie(items=[Rest('r4')])

Notice that the nontrivial keyword is effectively acting a type of filter: trivial output is excluded when nontrivial=True, and nontrivial output is excluded when nontrivial=False.

This may appear fine when you first look at it, but this really isn't the right interface to this functionality. Note that the output of nontrivial=None differs from nontrivial=False:

for logical_tie in abjad.select.logical_ties(voice, nontrivial=None):
    print(logical_tie)
LogicalTie(items=[Note("c'8"), Note("c'8")])
LogicalTie(items=[Note("d'8"), Note("d'8")])
LogicalTie(items=[Note("e'4")])
LogicalTie(items=[Rest('r4')])

Why? Because the meaning of nontrivial=None is something like "filter out neither trivial logical ties nor nontrivial logical ties; instead, return all logical ties to me without filtering." But we've got a keyword that looks like boolean (true/false) when it actually operates on three values (true/false/none) instead.

Here's the same pattern for the pitched keyword, and the grace keyword works similarly:

for logical_tie in abjad.select.logical_ties(voice, pitched=None):
    print(logical_tie)
LogicalTie(items=[Note("c'8"), Note("c'8")])
LogicalTie(items=[Note("d'8"), Note("d'8")])
LogicalTie(items=[Note("e'4")])
LogicalTie(items=[Rest('r4')])
for logical_tie in abjad.select.logical_ties(voice, pitched=True):
    print(logical_tie)
LogicalTie(items=[Note("c'8"), Note("c'8")])
LogicalTie(items=[Note("d'8"), Note("d'8")])
LogicalTie(items=[Note("e'4")])
for logical_tie in abjad.select.logical_ties(voice, pitched=False):
    print(logical_tie)
LogicalTie(items=[Rest('r4')])

So how to provide an unambiguous interface to this functionality?

I think the right thing to do is to replace each of these "tricky ternary" keywords with a pair of booleans. So the current (ternary) nontrivial will be replaced by (boolean) exclude_nontrivial together with (boolean) exclude_trivial; the current (ternary) pitched will be replaced by (boolean) exclude_pitched together with (boolean) exclude_nonpitched; the current (ternary) grace will be replaced by (boolean) exclude_grace together with exclude_nongrace; and so on:

OLD: abjad.select.logical_ties(..., nontrivial=True)
NEW: abjad.select.logical_ties(..., exclude_trivial=True)

OLD: abjad.select.logical_ties(..., nontrivial=False)
NEW: abjad.select.logical_ties(..., exclude_nontrivial=True)

OLD: abjad.select.logical_ties(..., pitched=True)
NEW: abjad.select.logical_ties(..., exclude_nonpitched=True)

OLD: abjad.select.logical_ties(..., pitched=False)
NEW: abjad.select.logical_ties(..., exclude_pitched=True)

etc.

The changes are straightforward, but I'll probably dedicate a release to just them since they'll mean that most users will have to refactor.

This should fix your original observation because there will no longer be competing ideas of what "trivial" means: trivial will mean trivial, and the interface keywords suppress trivial logical ties or nontrivial logical ties, (or all logical ties, if you set both exclude keywords to true).

Feel free to add other suggestions here.

@trevorbaca trevorbaca self-assigned this Sep 12, 2022
@trevorbaca trevorbaca changed the title Suggestion: harmonize nontrivial/is_trivial Refactor abjad.select keywords Sep 12, 2022
@martin-ribot
Copy link
Author

martin-ribot commented Sep 30, 2022 via email

@trevorbaca
Copy link
Member

Review (and probably link) #1123 when working this issue.

@trevorbaca trevorbaca added this to the 4.0 milestone Oct 12, 2022
@trevorbaca trevorbaca changed the title Refactor abjad.select keywords Refactor abjad.select() keywords Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants