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

Modernise and fix most notebooks #61

Open
wants to merge 10 commits into
base: gh-pages
Choose a base branch
from

Conversation

gijzelaerr
Copy link

@gijzelaerr gijzelaerr commented Nov 14, 2023

This PR:

  • Went through the notebooks and made compatible with the latest libraries listed in requirements.txt
  • There is a bug in librosa 0.10 which breaks the .showwave() function when used with matplotlib >3.8, so I fixed the version to matplotlib<3.8
  • I added a helper Makefile to quickly setup and open a juputer lab locally with the listed versions installed. Just run make, to start a notebook server. Should work on OSX and Linux, if python3 and make are installed.
  • I've cleared all cells and reran the notebook before committing.
  • Adds a nbmake github action to test the notebooks

Some notebooks have broken upstream dependencies or are example material, not sure what to do there:

FAILED adtlib.ipynb:: - ModuleNotFoundError: No module named 'ADTLib'
FAILED evaluation_beat.ipynb:: -     https://numpy.org/devdocs/release/1.20.0-notes.html#deprecations
FAILED evaluation_chord.ipynb:: - TypeError: evaluate() missing 4 required positional arguments: 'ref_intervals', 'ref_labels', 'est_intervals', and 'est_labels'
FAILED exercise_genre_recognition.ipynb:: - NameError: name 'mfcc_busta' is not defined

@gijzelaerr gijzelaerr changed the title fix chapter 1,2 & 3 Modernise and fix all notebooks Nov 15, 2023
@gijzelaerr gijzelaerr changed the title Modernise and fix all notebooks Modernise and fix most notebooks Nov 15, 2023
@iranroman
Copy link
Owner

Thank you! this is awesome. I will review soon.

@iranroman
Copy link
Owner

Hello @gijzelaerr, I've been going through the PR, and in general everything looks very good. Something that is unclear is whether you meant to include all the docker files. We currently do not support docker, so if that is a feature that you want to add, I recommend creating a different PR so that we can discuss. The notebook updates will be welcome otherwise.

@smith-kyle
Copy link

I'm a little late to the party, but I noticed you all aren't using a notebook review tool and wanted to invite you to review this pull request with GitNotebooks: https://gitnotebooks.com/iranroman/musicinformationretrieval.com/pull/61

It lets you do things like comment on rendered markdown and code cells, so might be an easy win for your PR reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants