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

Pylint pass for midi module #1946

Merged
merged 1 commit into from Sep 5, 2020
Merged

Pylint pass for midi module #1946

merged 1 commit into from Sep 5, 2020

Conversation

MyreMylar
Copy link
Contributor

Tested using: pylint src_py/midi.py

This PR reduces the pylint warnings down to this list:

src_py\midi.py:18:2: W0511: TODO: finish writing tests. (fixme)
src_py\midi.py:21:2: W0511: TODO: create a background thread version for input threads. (fixme)
src_py\midi.py:103:2: W0511: TODO: find all Input and Output classes and close them first? (fixme)
src_py\midi.py:377:45: W0613: Unused argument 'buffer_size' (unused-argument)

Three TODOs, which are beyond the scope of a pylint pass, and one genuine error in the public interface. It appears that the buffer_size parameter passed to the midi.Output class is never used at all.

I'll raise a separate issue about the buffer_size parameter.

Disabled:

def quit(): # pylint: disable=redefined-builtin

This is a sort of generic problem across all pygame modules - that their quit functions shadow python's built-in quit() function. For example, if you do:

from pygame.midi import *

quit()

You will call the pygame.midi modules quit() function rather than the built-in quit() function. I don't think we can change this at this point, all those module quit() functions are out there now.

@MyreMylar
Copy link
Contributor Author

I did some weird code to avoid having to use the global keyword for module level state here:

def _module_init(state=None):
    # this is a sneaky dodge to store module level state in a non-public
    # function. Helps us dodge using globals.
    if state is not None:
        _module_init.value = state
        return state

    try:
        _module_init.value
    except AttributeError:
        return False
    else:
        return _module_init.value

I'm not particularly happy about it, though it does work.

The standard advice for getting rid of mutable globals is to transform the code using them into a class, but that isn't really an option here because the midi module is already in widespread use. Thus we come back to this. Does anyone have a cleaner way to purge globals like this?

Copy link
Member

@illume illume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanups. Thanks 👍

@illume illume merged commit 80678ab into pygame:master Sep 5, 2020
@MyreMylar MyreMylar deleted the pylint-midi branch September 11, 2020 15:19
@illume illume added midi pygame.midi Linters Related to code linting issues or solutions and removed needs-review labels Sep 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Linters Related to code linting issues or solutions midi pygame.midi
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants