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

Improve management of bends #1471

Open
adhooge opened this issue Oct 19, 2022 · 2 comments · May be fixed by #1580
Open

Improve management of bends #1471

adhooge opened this issue Oct 19, 2022 · 2 comments · May be fixed by #1580

Comments

@adhooge
Copy link

adhooge commented Oct 19, 2022

Motivation

As of now (version 8.1.0), it seems that bends (the technique of bending a string to make the note go higher, widely used in guitar for instance) can only be added as an articulation to notes with music21.articulations.FretBend.

In the doc, this articulation only has a number attribute which seems inherited from articulations.FretIndication while many more attributes are needed. Looking at the code, I see that some other attributes are mentioned though not documented nor actually used:

# in music21/articulations.py
class FretBend(FretIndication):
    bendAlter: interval.IntervalBase | None = None
    preBend: t.Any = None
    release: t.Any = None
    withBar: t.Any = None

These are the attributes used in musicXML (see the doc) which is great so I guess what's only missing is to actually implement them in the musicXML reader/writer?

edit: as a side note, I also wonder how Pitch should be managed when a note has a bend. For reading purposes, as it is done in musicXML, it probably makes more sense to keep the pitch of the note that has no bend. But for analysis, like in music21, a note with a bend should probably be considered as having the pitch of the destination of the bend, not the pitch of its starting point. This is an open discussion, I'd love to hear your thoughts on that.

Feature summary

  • Complete the documentation on the FretBend articulation to mention all existing attributes (I can do this);
  • Complete the class code to use the attributes, parse them from musicXML etc. (I could probably do it with some help);
  • Change the class inheritance because a bend does not need to be a FretIndication, I think? Maybe a little complicated to change that and not that useful, I don't know;
  • Make bends interact nicely between tablature staves and standard staves, see following example from GuitarPro for instance. I have no idea how to implement it, probably the hardest thing to do but it would be nice that a bend in a tablature staff automatically shows the corresponding pitch change on the related staff. This is also kinda related to the discussion on the pitch given to a note with a bend (see edit above).
    image

Proposed implementation

To discuss

Intent

[x] I plan on implementing this myself.
[ ] I am willing to pay to have this feature added.
[x] I am starting a discussion with the hope that community members will volunteer their time to create this. I understand that individuals work on features of interest to them and that this feature may never be implemented.

@adhooge
Copy link
Author

adhooge commented Nov 24, 2022

Hello again,

I added a few features to my music21 to deal with bends, I'm not quite sure that it is in line with music21 code guidelines but I can improve it for a future PR if it can be useful to anyone.

I won't write everything here so see my fork for complete details. Essentially I modified m21ToXml.py and added bend support with:

if musicXMLTechnicalName == 'bend':
            bendAlterSubElement = SubElement(mxTechnicalMark, "bend-alter")
            bendAlterSubElement.text = str(articulationMark.bendAlter.semitones)
            if articulationMark.preBend:
                preBendSubElement = SubElement(mxTechnicalMark, "pre-bend")
            if articulationMark.release is not None:
                releaseSubElement = SubElement(mxTechnicalMark, "release")
                releaseSubElement.set("offset", str(articulationMark.release))
            if articulationMark.withBar is not None:
                withBarSubElement = SubElement(mxTechnicalMark, "with-bar")
                withBarSubElement.text = str(articulationMark.withBar)

And I properly defined the FretBend articulation like so:

class FretBend(FretIndication):
    bendAlter: interval.IntervalBase | None = None
    preBend: t.Any = None
    release: t.Any = None
    withBar: t.Any = None

    def __init__(self, number=0, preBend=False, release=None, withBar=None, **keywords):
        '''
        bend indication for fretted instruments

        bend in musicxml

        number is the interval of the bend in number of semitones, bend-alter in musicxml
        preBend indicates wether the note is prebended or not
        release is the offset value for releasing the bend, if Any
        withBar indicates if the bend is done using a whammy bar movement
        '''
        super().__init__(**keywords)
        self.bendAlter = interval.ChromaticInterval(number)
        self.preBend = preBend
        self.release = release
        self.withBar = withBar

This is enough for what I currently want to do, I'd be happy to explain what it does and how I do it if it helps :)

@mscuthbert
Copy link
Member

Happy to take it as a PR, though there's no need to give type for class members -- it can be done in the __init__ and t.Any is too broad; we're requiring typing for all new arguments etc.

For the doc building make sure to put an empty blank space between each member element.

For a PR, please also do xmlToM21.py as well -- I really want everything to round trip. (and single-quotes unless double are needed.

Thanks! A great contribution.

@adhooge adhooge linked a pull request May 23, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants