Policy/Guide on *args, **keywords

[mscuthbert]

Motivation

As music21 has grown, the ad hoc way in which *args, **kwargs, etc. has been treated has led to gaps in documentation, expectations for users, etc., leading to issues such as (#839) etc. This Issue is a proposed solution moving forward.

Proposed guidance
Proposed are the following:

1. All v7 functions can continue as is but should be updated when possible. The existence of pre-v8 functions that violate these rules cannot be cited as precedent.
2. (statement of current rules, with rewording): functions can have no more than 3 positional arguments, and should only include ones used often.
   • methods may have "self" + 3 positional arguments
   • remaining arguments should be keyword only
   • when adding new functionality to existing functions, nearly always, a keyword-only argument is the right choice.
   • the order of arguments should make sense: numerator -> denominator, from -> to. If it's likely to be confused (especially if two variables are the same type), better to use keyword only.
3. For arbitrary positional and keyword arguments that are not parsed, the names are *arguments, **keywords. Explicit Keywords on all music21 objects #1377 has made this rename for **keywords -- this change was easy since **kwargs is daunting to new users (I remember not knowing what it meant). The choice of *args vs *arguments was tough, since *args is more obvious what it means than **kwargs, and *arguments is longer; they are used about equally in existing code. The decision was based on *arguments being more explicit, AND being longer to type is a slight ADVANTAGE since this Issue discourages using it. And, this should be a moot issue in any case, because of changes below:
4. Functions should generally NOT look at **keywords. (Keywords need to be part of the function signature #839) -- any members that will be used in the function should be explicitly mentioned in the function signature.
   • So instead of if 'myOption' in keywords... put myOption: t.Optional[Something] = None in the signature of the function.
   • An exception is granted for for multiple inheritance to examine the **keywords dict to split it for two different constructors which use the same key to mean something different.
5. Methods (especially constructors for objects subclassing Music21Object, Stream, Note, etc.) can have a generic **keywords at the end of the constructor signature which is assumed to be passed up the inheritance chain to the superclass for its own keywords (or its superclass's own keywords, etc.). This way other Music21Objects don't have to deal with duration, groups, style, etc. and can pass it up the chain. Especially important for id since this requires pylint exemptions, etc.
6. The only use of (*arguments, **keywords) is for a quick subclass that needs to keep track of something else along the way and we want to avoid the maintenance burden of, e.g., updating Marcato whenever the signature of Music21Object, Articulation, etc. changes. Core and often used subclasses should know what the signatures of their superclasses are and write their constructors accordingly. The use of common.types shortcuts should be used to avoid writing long and variable types over and over. Even here, don't use *arguments if the superclass does not have arguments currently.
7. Aside from the case above *arguments, *args should only be used in cases where the number of arguments is actually unlimited and all of the same type as each other and have the same meaning. E.g., in Chord, where an arbitrary number of Note arguments can be passed in separated by commas. Code should not have things like what Tuplet currently has, where *arguments can only have 2 or 3 entries and the first means numNotesActual: int, the second numNotesNormal: int, and the third durationActual: str|DurationTuple. These can be normal arguments which can be specified by position or keyword.
8. When *arguments are used properly (like in Chord) the variable name should reflect the type of thing expected, e.g., *notes. (Or in the case of Chord which can also take a list of notes, (self, noteListOrFirstNote, *remainingNotes) )
9. There is currently no reason to have positional-only arguments, so don't use them, even now that Python 3.8 is the minimum version so it's supported everywhere. use positional-only arguments only when there is a variable type argument like "noteOrStringOrListOfNotes" which you'd rather no one type, AND there are keyword alternates like n=..., s=..., nList=... so that people can specify what they are putting in. Interval() is being refactored to use them.
10. Document all arguments in the docstring. Arguments that become stored in attributes (not properties) should be documented and tested in _DOC_ATTR = {}. Those that are manipulated and not stored directly should be tested in the docstring or a unittest.

Intent

[x] I plan on implementing this myself.

[mscuthbert]

And...no sooner did I create this, but I found a use for positional only arguments (first two arguments in Interval constructor), so just shows how in flux this can be.