MIDI-122: MidiPiece-Improvements

[SamuelJanas]

MidiPiece Improvements
Unit tests
packaging and releases

[SamuelJanas]

Looking at the trimming functionality I was thinking about 2 possible changes:

• I believe it should be possible to trim notes in other ways as well:
  1. trim given * index* interval
  2. trim excluding notes that end after the finish interval.
• The returned MidiPiece is treated as a separate piece; meaning the start of the new first note is set to 0. We should change that/add a flag to return either the new piece or a part of it. This could work nicely with the appending functionality(?)

Example:

trimmed_midi_piece = midi_piece.trim(1, 4)

Returns: 0, 1.0, 1.0, 62, 80 (start, end, duration, pitch, velocity). Instead of 1, 2.0, 1.0, 62, 80

LMK your opinion. For now I left it as is.

[SamuelJanas]

It isn't possible to vectorizem to_midi function, due to how PrettyMIDI works. I've managed to cut down time drastically using NumPy. For smaller pieces, the difference is unnoticeable. However, when the piece has more notes (e.g. 500 and more), it's approx. 10x faster:
24 ms ± 147 µs per loop (mean ± std. dev. of 3 runs, 100 loops each) - Numpy, 5000 notes.
207 ms ± 2.78 ms per loop (mean ± std. dev. of 3 runs, 100 loops each) - iterrows, 5000 notes

[roszcz]

Looking at the trimming functionality I was thinking about 2 possible changes:

• I believe it should be possible to trim notes in other ways as well:

  1. trim given * index* interval
  2. trim excluding notes that end after the finish interval.

• The returned MidiPiece is treated as a separate piece; meaning the start of the new first note is set to 0. We should change that/add a flag to return either the new piece or a part of it. This could work nicely with the appending functionality(?)

Example:

trimmed_midi_piece = midi_piece.trim(1, 4)

Returns: 0, 1.0, 1.0, 62, 80 (start, end, duration, pitch, velocity). Instead of 1, 2.0, 1.0, 62, 80

LMK your opinion. For now I left it as is.

By trimming with index interval, do you mean a similar functionality to what __geitem__ is doing, but without the time shift?

I think we can add an argument like shift_time: bool = True, it could work for both index and time based trimming.

Thinking about adding pieces - I'd say that doing something like this can be tricky:

piece_a[3: 10] + piece_b[120:130]

You could do this without any time shifts, or by shifting the second piece to start right after the first one. Either way it's difficult to define so it makes musical sense - I'm sort of thinking about throwing an error in __add__ with information that the user should take care to join dataframes with notes and implement their own time control 🤔

[SamuelJanas]

By trimming with index interval, do you mean a similar functionality to what geitem is doing, but without the time shift?

That's correct. I'll add the argument, shouldn't make much difference but maybe someone will need it one day. 🙏

You could do this without any time shifts, or by shifting the second piece to start right after the first one. Either way, it's difficult to define so it makes musical sense

I see what you mean. I don't think throwing an error would be 'user-friendly' I suppose that letting the user do what they want, disregarding the musicality could be a good thing. It sort of depends on whether we want Fortepyan to be your friendly music helper 😄, or a tool you use for working with MIDI from a more technical standpoint. With the second option the harsh "join two pieces as is" would work as an editor.

To connect those ideas maybe throwing a warning would do? Something along the lines of "Hey it's connected but are you sure this is what you want?" 😅

[roszcz]

Yes, warning will be good for this 👍

Generally speaking I'd say MidiPiece class should go more in the direction of a "tool you use for working with MIDI from a more technical standpoint" :)

[SamuelJanas]

I'm following this documentation guide as well as material mkdocs documentation to create some baseline of how our documentation could look like. I'll be doing some research on good practices and will try to apply them to Fortepyan.

[roszcz]

@SamuelJanas Thanks! For reference here's a couple of well documented projects:

• https://stripe.com/docs/api
• https://platform.openai.com/docs/api-reference/introduction
• https://docs.pydantic.dev/latest/
• https://fastapi.tiangolo.com/reference/

[SamuelJanas]

I've looked at the projects you linked. Pydantics seems like the most similar project to ours documentation-wise. I've tried to showcase how it could be developed further with this one page that we have.
You can run it locally with mkdocs serve.
Because of mkdocstrings the documentation writes itself and has better quality if the docstrings of the functions have better quality. I wrote excessive documentation for most methods in MidiPiece. I'll try to automate it given the structure I tried to keep. 😅

I was wondering which Example section to keep. Here's a comparison of two ideas I had.

[roszcz]

Is there a way to generate the documentation automatically from docstrings, and also add images in mkdocs? Showing piano rolls before/after could be very helpful for some operations.

I'm also thinking about adding the JS piano roll player to our documentation, I guess it's also going to cause some problems with the automatic docstring-documentation conversion 🤔

[SamuelJanas]

Is there a way to generate the documentation automatically from docstrings, and also add images in mkdocs? Showing piano rolls before/after could be very helpful for some operations.

I believe it's treating the docstrings similarly to markdown so there might be a way. I'll inspect how it could be done. But I was thinking about including a separate page dedicated to examples and visualisations. Pydantic has a similar page that's under construction. This way we could write a more detailed description for the charts without cluttering the technical aspect of the documentation.

[SamuelJanas]

Shortly speaking, it's possible to add an image to the documentation. relative links work just fine, it is, however, a little tricky to guess what will be the final path.

mkdocstrings/mkdocstrings#456 Issue I found the answer in.

[SamuelJanas]

This PR is complete. Some improvements were made to MidiPiece. We have the beginning of technical docs and CI/CD has been set up. LMK if you feel like anything else is needed here.

[roszcz]

Thanks! 💪

Can you propose and document a version bumping mechanism for this? Up till now I used to do:

bumpver update --patch
python -m build
twine upload -r pypi dist/*

We have those settings for bumpver in pyproject.toml:

[tool.bumpver]
current_version = "0.2.4"
version_pattern = "MAJOR.MINOR.PATCH"
commit_message = "bump version {old_version} -> {new_version}"
commit = true
tag = true
push = true

The commit, tag, and push combination is doing a lot, and I'm guessing it's not consistent with the gitflow apporach we were discussing. Can you propose a new approach for this? I like using bumpver, so maybe it would make sense to use it during git flow release, but without commit, tag, and push 🤔

[roszcz]

Looks like this is some generic template, can you make it fortepyan-specific? Explicit instructions on how to get the documentation running locally would be great, including required dependencies. I almost got mkdocs serve to work, but I'm stuck on:

ModuleNotFoundError: No module named 'mkdocstrings_handlers'

[SamuelJanas]

I'll try running it on a fresh environment and note what needs to be done! Sorry for inconvenience.

[SamuelJanas]

Thanks! 💪

Can you propose and document a version bumping mechanism for this? Up till now I used to do:

bumpver update --patch
python -m build
twine upload -r pypi dist/*

We have those settings for bumpver in pyproject.toml:

[tool.bumpver]
current_version = "0.2.4"
version_pattern = "MAJOR.MINOR.PATCH"
commit_message = "bump version {old_version} -> {new_version}"
commit = true
tag = true
push = true

The commit, tag, and push combination is doing a lot, and I'm guessing it's not consistent with the gitflow apporach we were discussing. Can you propose a new approach for this? I like using bumpver, so maybe it would make sense to use it during git flow release, but without commit, tag, and push 🤔

I was thinking about it. I've managed to come up with a solution that would keep the minimal commands to write at 2.

That would include creating two shell scripts. Something along the lines of release_start.sh and release_finish.sh. The first would take an argument similarly to bumpver (patch, minor, major). It would run bumpver (the most barebone version that would simply bump the version). Then we would run git flow release start $new_version and print some information like "Release $new_version started. You can now make any last-minute changes.".

release_finish.sh on the other hand would run git flow release finish with appropriate tag commit message and all the necessary pushes (origin develop, origin main, --tags). I feel like this could work fine?

The rest would be handled by github actions. I'll change the setup.py (totally missed that) to pyproject.toml right after I do some testing. 👍

[roszcz]

Sounds good, we can try that! Maybe we could store those scripts in scripts/release?

[SamuelJanas]

Quick update here. I've written the scripts, I was able to test them on windows only so if you get that weird end of the line error coming from different EOL in windows and linux then I'm terribly sorry! 😞

Looks like this is some generic template, can you make it fortepyan-specific? Explicit instructions on how to get the documentation running locally would be great, including required dependencies. I almost got mkdocs serve to work, but I'm stuck on:

After running:

pip install -r requirements.txt
pip install mkdocs-material
pip install mkdocstrings-python

on a fresh environment, I was able to run mkdocs serve. Should I add those two libraries to requirements.txt?
Apart from that I suppose we could merge this PR and try out our new scripts on a patch to fortepyan 😄

[roszcz]

Quick update here. I've written the scripts, I was able to test them on windows only so if you get that weird end of the line error coming from different EOL in windows and linux then I'm terribly sorry! 😞

Looks like this is some generic template, can you make it fortepyan-specific? Explicit instructions on how to get the documentation running locally would be great, including required dependencies. I almost got mkdocs serve to work, but I'm stuck on:

After running:

pip install -r requirements.txt
pip install mkdocs-material
pip install mkdocstrings-python

on a fresh environment, I was able to run mkdocs serve. Should I add those two libraries to requirements.txt? Apart from that I suppose we could merge this PR and try out our new scripts on a patch to fortepyan 😄

Thanks, docs work for me as well after this, I probably messed something up previously 🙈 I think we can wait with figuring out the docs dependencies until we get to deployment setup.