search

urinieto / msaf

Music Structure Analysis Framework
MIT License
478 stars 79 forks source link

Fix OSError in tests #143

Closed carlthome closed 9 months ago

carlthome commented 9 months ago

Recent matplotlib and seaborn produce

ERROR test_run.py - OSError: 'seaborn-ticks' is not a valid package style, path of style file, URL of style file, or library style name (library styles are listed in `style.available`)

when running the tests.

This PR works around that by simply not setting that style anymore. Perhaps more digging would be prefered though? Let me know and I'll try to understand this error better!

Also made sure import happens first in the test modules. Just a minor thing. Can remove this if prefered!

carlthome commented 9 months ago

Hmm, so interesting follow up issues in https://github.com/urinieto/msaf/actions/runs/6339872339/job/17219993213?pr=143#step:6:539 here for Python 3.9 and 3.10 (but not 3.8). Checking!

carlthome commented 9 months ago

This seems like a deep rabbit hole with how NumPy dispatches functions. Since the assertion is just to check that byte code is equivalent, perhaps it's easiest to change the assertion to check that the functions are the same instead. Thoughts?

Related: https://github.com/numpy/numpy/issues/24019

carlthome commented 9 months ago

Hmm, still one error to go:

FAILED test_features.py::test_no_audio - msaf.exceptions.NoAudioFileError: Computation of the features is needed for current parameters but no audio file was found.Please, change your parameters or add the audio file in fixtures/chirp_noaudio.mp3

urinieto commented 9 months ago

Nice reorg of the imports. Regarding the error, I'm partial to the potential solution, so feel free to do what you think it's best in this case. I'll review once tests pass.

carlthome commented 9 months ago

Looks like np.max has changed __name__ in NumPy from amax to max.

>>> import numpy as np
>>> np.__version__
'1.19.5'
>>> np.max.__name__
'amax'
>>> import numpy as np
>>> np.__version__
'1.26.0'
>>> np.max.__name__
'max'

so this check fails https://github.com/urinieto/msaf/blob/main/msaf/base.py#L245-L246

carlthome commented 9 months ago

https://github.com/numpy/numpy/commit/10743b0bcfa0905b15ccadfe164cfdb57ad6cf6a possible cause

carlthome commented 9 months ago

Alright, think this is ready to go in now to fix the unit tests after going through Matplotlib / NumPy updates.

Let me know if you want to break up this into smaller PRs first!