ShazamAPI icon indicating copy to clipboard operation
ShazamAPI copied to clipboard

Refactoring

Open notpushkin opened this issue 3 years ago • 9 comments

This is a big PR, but if you go commit-by-commit it should be approachable.

Poetry

I've switched to https://python-poetry.org/ for dependency (+ virtualenv) management, you'll want to install it. Note that Poetry isn't required for users to install it when you release it to PyPI. Once you have installed it, run poetry install to get the dependencies.

Working with linter

To see all the "violations" currently in the project, run:

poetry run flakeheaven lint --baseline /dev/null

This will result in around 200 nits, mostly about code complexity and magic numbers. I don't think refactoring all this is feasible in one go – this is why I've set up a baseline for the linter. If you run it like this:

poetry run flakeheaven lint

right now you'll see zero errors. However, if you happen to change anything, it will force you to refactor this part of code as well.

I'm not sure what's the approach will work for you, but I'll be glad if it helps even a bit!

Next steps

  • Publish to PyPI (poetry publish --build should do the trick)

  • Write up some more docs, perhaps as Sphinx site on ReadTheDocs?

  • Write up more examples (e. g. what if we use sounddevice to record audio, then convert it to an AudioSegment and pass to recognize_song?)


Closes #7

notpushkin avatar Apr 03 '22 22:04 notpushkin

Really great improvements. What about minimum python version, is it really need 3.8+ or we can make it possible to use even on 3.7? I still actually use this version :)

Numenorean avatar Apr 04 '22 21:04 Numenorean

I think 3.8 is needed for some of the devDependencies – I can see if there's any way around this.

Note that EOL for 3.7 is next June – you have just slightly more than a year to upgrade :-)

notpushkin avatar Apr 05 '22 10:04 notpushkin

Turns out NumPy has also dropped Python 3.7 support starting with the 1.22.0 release. I've downgraded it to ^1.21.1 for the time being, and made pydub-stubs and ipython optional dependencies.

notpushkin avatar Apr 05 '22 10:04 notpushkin

oops, it seems typing.Final has been released in python 3.8. As i don't have too much experience in python, what do you think about support for 3.7, do we need it here?

Numenorean avatar Apr 05 '22 18:04 Numenorean

I'm not sure – that's totally up to you :D We can get rid of Finals though, they're here only for type checker strictness. Another option is to use typing_extensions.Final.

notpushkin avatar Apr 07 '22 10:04 notpushkin

i guess typing_extensions.Final would be better

Numenorean avatar Apr 07 '22 11:04 Numenorean

I've added typing-extensions on Python < 3.8. Unfortunately, I can't get my hands on Python 3.7 easily, to try it out and see if anything else breaks:

$ brew install [email protected]
[email protected]: The x86_64 architecture is required for this software.

Wanna take over? You have commit access to my fork, git pull [email protected]:notpushkin/ShazamAPI.git refactor:notpushkin/refactor && git checkout notpushkin/refactor should do the trick :-)

notpushkin avatar Apr 07 '22 14:04 notpushkin

On a slightly different note, here's how you can do the microphone recognition: https://codeberg.org/notpushkin/shazamctl/src/branch/master/shazamctl/microphone.py 🎉

notpushkin avatar Apr 17 '22 10:04 notpushkin

@Numenorean hey! Is there anything left before we can merge this in?

notpushkin avatar Jun 03 '23 11:06 notpushkin