-
Notifications
You must be signed in to change notification settings - Fork 433
Prepare Music21 to run on Ruff #1846
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
Conversation
|
Requested review was just to see if Jacob had experience with Ruff to weigh in with -- not expecting anyone to review 200+ file changes! |
Wow was that not working the way it was supposed to!
|
Cool, I'll be glad to hear what you think of ruff, I haven't toyed with it myself. I know pylint uses ruff :-) My "wirecutter" review is that the reason pylint isn't completely dead yet is that it collects the values variables (allegedly) take and uses that to find bad calls (most useful for untyped projects) and also flags undefined names (ruff doesn't have this). If you had students working on this code, or were often working in fragile areas, I could see that being useful. But music21 has been using mypy for a while now. Happy to hear how it goes if you swap it out! |
| * Changed in v10: Remove part about creating an Unpitched object -- that has been wrong | ||
| for some time (probably since PercussionChords were introduced) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds juicy. What happened here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It appears that there used to be a place where MidiEvents were turned into either Notes or Unpitched objects in this routine. But at some point the decision about what they should become was moved up one level without altering the docs. It meant that the main (first) argument, MidiEvent, was never used!
Oh? Uses Ruff in evaluating its own codebase? It doesn't actually import Rust itself?
Ah -- I see now -- Ruff has basically replaced flake8 and can find a lot of pylint-area errors fast, so it's worth running before running Pylint, but doesn't fully replace it. I just wish they shared the same "disable" code! :-) So I think the next step is to add Ruff to the build chain, replacing flake8, and then start turning off pylint checks that Ruff can also handle, leaving it to do what it does best. (Happy New Year Jacob! Hope everything is amazing with you!) |
So much faster! But needed some cleanup first.
Remove nearly all % and format code for f-strings too.