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.

Sign up for GitHub

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

Feature/figuredbass object #1614

Merged
merged 10 commits into from
Jul 2, 2023
Merged

Feature/figuredbass object #1614

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b7ae781
introduces FiguredBass object and makes some changes to Notation and …
Jun 19, 2023
94f76b4
indentation update
Jun 19, 2023
d6ee784
corrections to notation PR #1614
Jun 20, 2023
7b099f2
added requested changes to figuredbass and notation object
Jun 21, 2023
c4609bf
first commit for fixing issues in place.
mscuthbert Jul 1, 2023
c847113
move FiguredBass object to figuredBass.FiguredBass
mscuthbert Jul 2, 2023
2672974
Merge branch 'master' into pr/1614
mscuthbert Jul 2, 2023
70f9f2c
lint and mypy
mscuthbert Jul 2, 2023
f232f9d
Cover uncovered test lines. https://coveralls.io/builds/60910555
mscuthbert Jul 2, 2023
88b8ed9
reuse your setters, Myke!
mscuthbert Jul 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 138 additions & 4 deletions music21/figuredBass/notation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,33 @@
(2,): (6, 4, 2),
}

prefixes = ['+', '#', '++', '##']
suffixes = ['\\']

modifiersDictXmlToM21 = {'sharp': '#',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indent as

modifiersDictXmlToM21 = {
    'sharp' :'#',
    'flat': 'b',
    ...etc.
}

so that all keys begin on the same column and have four spaces showing they're inside a top-level dictionary. (same below)

'flat': 'b',
'natural': '\u266e',
'double-sharp': '##',
'flat-flat': 'bb',
'backslash': '\\',
'slash': '/',
'cross': '+'}

modifiersDictM21ToXml = {'#': 'sharp',
'b': 'flat',
'##': 'double-sharp',
'bb': 'flat-flat',
'\\': 'backslash',
'/': 'slash',
'+': 'sharp',
'\u266f': 'sharp',
'\u266e': 'natural',
'\u266d': 'flat',
'\u20e5': 'sharp',
'\u0338': 'slash',
'\U0001D12A': 'double-sharp',
'\U0001D12B': 'flat-flat',
}

class Notation(prebase.ProtoM21Object):
'''
Expand Down Expand Up @@ -79,6 +106,7 @@ class Notation(prebase.ProtoM21Object):

* '13' -> '13,11,9,7,5,3'

* '_' -> treated as an extender

Figures are saved in order from left to right as found in the notationColumn.

Expand Down Expand Up @@ -139,6 +167,23 @@ class Notation(prebase.ProtoM21Object):
<music21.figuredBass.notation.Figure 6 <Modifier b flat>>
>>> n3.figures[1]
<music21.figuredBass.notation.Figure 3 <Modifier b flat>>
>>> n3.extenders
[False, False]
>>> n3.hasExtenders
False
>>> n4 = notation.Notation('b6,\U0001D12B_,#')
>>> n4.figures
[<music21.figuredBass.notation.Figure 6 <Modifier b flat>>,
<music21.figuredBass.notation.Figure _ <Modifier 𝄫 double-flat>>,
<music21.figuredBass.notation.Figure 3 <Modifier # sharp>>]
>>> n4.figuresFromNotationColumn
[<music21.figuredBass.notation.Figure 6 <Modifier b flat>>,
<music21.figuredBass.notation.Figure _ <Modifier 𝄫 double-flat>>,
<music21.figuredBass.notation.Figure None <Modifier # sharp>>]
>>> n4.extenders
[False, True, False]
>>> n4.hasExtenders
True
'''
_DOC_ORDER = ['notationColumn', 'figureStrings', 'numbers', 'modifiers',
'figures', 'origNumbers', 'origModStrings', 'modifierStrings']
Expand Down Expand Up @@ -189,12 +234,15 @@ def __init__(self, notationColumn=None):
self.origModStrings = None
self.numbers = None
self.modifierStrings = None
self.extenders:list[bool] = []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space before list[bool]

self.hasExtenders: bool = False
self._parseNotationColumn()
self._translateToLonghand()

# Convert to convenient notation
self.modifiers = None
self.figures = None
self.figuresFromNotationColumn: list(Figure) = []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

list[Figure] -- square brackets not parentheses.

self._getModifiers()
self._getFigures()

Expand Down Expand Up @@ -224,11 +272,19 @@ def _parseNotationColumn(self):
(6, None)
>>> notation2.origModStrings
('-', '-')

hasExtenders is set True if an underscore is parsed within a notation string
>>> notation2.hasExtenders
False

>>> notation3 = n.Notation('7_')
>>> notation3.hasExtenders
True
'''
delimiter = '[,]'
figures = re.split(delimiter, self.notationColumn)
patternA1 = '([0-9]*)'
patternA2 = '([^0-9]*)'
patternA1 = '([0-9_]*)'
patternA2 = '([^0-9_]*)'
numbers = []
modifierStrings = []
figureStrings = []
Expand All @@ -247,17 +303,40 @@ def _parseNotationColumn(self):

number = None
modifierString = None
extender = False
if m1:
number = int(m1[0].strip())
# if no number is there and only an extender is found.
if '_' in m1:
self.hasExtenders = True
number = '_'
extender = True
else:
# is an extender part of the number string?
if '_' in m1[0]:
self.hasExtenders = True
extender = True
number = int(m1[0].strip('_'))
else:
number = int(m1[0].strip())
if m2:
modifierString = m2[0].strip()

numbers.append(number)
modifierStrings.append(modifierString)
self.extenders.append(extender)

numbers = tuple(numbers)
modifierStrings = tuple(modifierStrings)

# extenders come from the optional argument when instantionting the object.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo. instantiating.

# If nothing is provided, no extenders will be set.
# Otherwise we have to look if amount of extenders and figure numbers match
#
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove blank #

if not self.extenders:
self.extenders = [False for i in range(len(modifierStrings))]
else:
extenders = tuple(self.extenders)

self.origNumbers = numbers # Keep original numbers
self.numbers = numbers # Will be converted to longhand
self.origModStrings = modifierStrings # Keep original modifier strings
Expand Down Expand Up @@ -366,11 +445,26 @@ def _getFigures(self):
for i in range(len(self.numbers)):
number = self.numbers[i]
modifierString = self.modifierStrings[i]
if self.extenders:
if i < len(self.extenders):
extender = self.extenders[i]
else:
extender = False
figure = Figure(number, modifierString, extender)
figure = Figure(number, modifierString)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is immediately wiped out -- mistake? Not tested enough.

figures.append(figure)

self.figures = figures

figuresFromNotaCol = []

for i, number in enumerate(self.origNumbers):
modifierString = self.origModStrings[i]
figure = Figure(number, modifierString)
figuresFromNotaCol.append(figure)

self.figuresFromNotationColumn = figuresFromNotaCol


class NotationException(exceptions21.Music21Exception):
pass
Expand All @@ -395,6 +489,20 @@ class Figure(prebase.ProtoM21Object):
'+'
>>> f1.modifier
<music21.figuredBass.notation.Modifier + sharp>
>>> f1.hasExtender
False
>>> f1.isExtender
False
>>> f2 = notation.Figure(6, '#', extender=True)
>>> f2.hasExtender
True
>>> f2.isExtender
False
>>> f3 = notation.Figure(extender=True)
>>> f3.isExtender
True
>>> f3.hasExtender
True
'''
_DOC_ATTR: dict[str, str] = {
'number': '''
Expand All @@ -410,15 +518,34 @@ class Figure(prebase.ProtoM21Object):
associated with an expanded
:attr:`~music21.figuredBass.notation.Notation.notationColumn`.
''',
'hasExtender': '''
A bool value that indicates whether an extender is part of the figure.
It is set by a keyword argument.
''',
'isExtender': '''
A bool value that returns true if an extender is part of the figure but no
number is given. Pure extender if you will.
It is set by evaluating the number and extender arguments.
'''
}

def __init__(self, number=1, modifierString=None):
isExtender: bool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove -- not a class property.


def __init__(self, number=1, modifierString=None, extender=False):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to:

def __init__(self, number=1, modifierString=None, *, extender: bool = False):

so that extender has to be given as a keyword. Otherwise people will do this in code:

n = notation.Figure(3, '#', True)

and I'll be scratching my head about what the True means.

self.number = number
self.modifierString = modifierString
self.modifier = Modifier(modifierString)
# look for extender's underscore
self.hasExtender: bool = extender
self._updateIsExtenderProperty()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove -- see below.


def _updateIsExtenderProperty(self):
self.isExtender = (self.number == 1 and self.hasExtender)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what I meant is if I create a figure object like:

>>> n = notation.Figure(1, '#', extender=True)

then the .isExtender property can get out of date:

>>> n.isExtender
True
>>> n.hasExtender = False
>>> n.isExtender
True

instead don't store isExtender anywhere and recalculate it on the fly. Let's rename it
to pureExtender or isPureExtender since that makes it clear what the difference with
hasExtender is:

@property
def pureExtender(self):
    '''
    Read-only boolean property that returns True if an extender is part of the 
    figure but no number is given (a number of 1 means no-number). It is a pure extender.

    >>> n = notation.Figure(1, '#', extender=True)
    >>> n.isExtender
    True
    >>> n.number = 2
    >>> n.isExtender
    False
    '''
    return self.number == 1 and self.hasExtender

and then it will always be up-to-date no matter what.

note -- should a pureExtender also require notationString to be '' or None?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Not storing this seperatly it makes sense. Thanks for adding this. I will include it.


def _reprInternal(self):
mod = repr(self.modifier).replace('music21.figuredBass.notation.', '')
if self.isExtender or self.hasExtender:
return f'{self.number} {mod} extender: {True}'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isExtender and hasExtender is redundant, right? since there should never be a case where something hasExtender but isExtender is False.

Maybe:

def _reprInternal(self):
    if self.isPureExtender:
        return 'pure extender'
    mod = repr(self.modifier).replace('music21.figuredBass.notation.', '')
    extender = '___' if self.hasExtender else ''
    return f'{self.number} {mod}{extender}'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where something hasExtender but isExtender is False

This is possible but the other way round it should not be the case. If isExtender is True hasExtender has to be True as well. I'm adding something very similar.

return f'{self.number} {mod}'


Expand All @@ -433,6 +560,13 @@ def _reprInternal(self):
'++': '##',
'+++': '###',
'++++': '####',
'\u266f': '#',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...a place where existing code doesn't match the modern style mentioned above. :-O ah well, can't be perfect. We didn't have good linters in 2008

'\u266e': 'n',
'\u266d': 'b',
'\u20e5': '#',
'\u0338': '#',
'\U0001d12a': '##',
'\U0001d12b': '--'
}


Expand Down
52 changes: 52 additions & 0 deletions music21/harmony.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
from music21 import environment
from music21 import exceptions21
from music21.figuredBass import realizerScale
from music21.figuredBass import notation
from music21 import interval
from music21 import key
from music21 import pitch
Expand Down Expand Up @@ -2499,6 +2500,57 @@ def transpose(self: NCT, _value, *, inPlace=False) -> NCT | None:

# ------------------------------------------------------------------------------

class FiguredBass(Harmony):
'''
The FiguredBassIndication objects store information about thorough bass figures.
It is created as a representation for <fb> tags in MEI and <figured-bass> tags in MusicXML.
The FiguredBassIndication object derives from the Harmony object and can be used
in the following way:
>>> fb = harmony.FiguredBass('#,6#', correspondingPart='P1')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space before.

>>> fb
<FiguredBass figures: #,6# correspondingPart: P1>

The single figures are stored as figuredBass.notation.Figure objects:
>>> fb.figNotation.figures[0]
<music21.figuredBass.notation.Figure 3 <Modifier # sharp>>
'''


def __init__(self,
figureString: str | list[str] | None = None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After music21 moved to the world of typing extensions (around v6) figureString should just take a string. If someone wants to pass in a list of figures that can be a different keyword-only argument like figureStrings.

There are lots of cases in the current code that haven't moved to this paradigm because they're grandfathered in, but we're trying to make it so that new code mostly takes one type of object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would then add the two keywords figureString and figureStrings. If nothing at all is handed over, it will result in an empty FiguredBass.

correspondingPart: str | None=None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, I wasn't clear about this. Anything ending in Part (part, correspondingPart, activePart, etc.) looks like it should be a Part object. partId would be clear that it is a string or None.

But I don't know why this would need this information at all -- the Part that an object is part of can always be retrieved by:

fb.getContextByClass(stream.Part)

then if the Figure is moved to a new Part or there are two parts with the same id, the FiguredBass will always be with the right part. This is why Note, Clef, etc. don't need a correspondingPart object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I understand.

Having a score where figured bass is not tied to one single staff/part I thought it might be a good way to keep the information close to the figures. And my intention was to store it in way that could be looked up fast.

But I'm seeing that this might not be necessary. I will remove that.

**keywords):
super().__init__(**keywords)

self.isFigure: bool = True
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed?

self.corresPart: str | None = None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's moot if we go the route above, but no abbreviations.

self._figs: str = ''

if figureString:
if isinstance(figureString, list):
self._figs = ','.join(figureString)
elif isinstance(figureString, str):
if ',' in figureString:
self._figs = figureString
else:
self._figs = ','.join(figureString)
else:
self._figs = ''
self._figNotation = notation.Notation(self._figs)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs typing.

self.corresPart = correspondingPart

@property
def figNotation(self) -> notation.Notation:
return self._figNotation

@figNotation.setter
def figNotation(self, figs: str | list[str] | None):
self._figNotation = notation.Notation(figs)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as noted above, this doesn't pass reflexive setters. (also no abbreviations. This should just be notation)

Needs docs and tests.

it seems like there should be two sets of properties. One for getting/setting the Notation object, and one for getting/setting the string of figures -- changing one should change the other.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. I'm pushing a proposal.


def __repr__(self):
return f'<{self.__class__.__name__} figures: {self.figNotation.notationColumn} correspondingPart: {self.corresPart}>'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Music21Objects don't define __repr__ just _reprInternal, which would be just the part from figures: {...} to the end before > -- let the Music21Object __repr__ take care of the class name, etc.

Generally the representation just provides the most relevant information, so I think it could just be return self.notation.notationColumn. in .show('text') it'll be obvious what Part the FiguredBass object belongs to.


# ------------------------------------------------------------------------------

def realizeChordSymbolDurations(piece):
'''
Expand Down