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

Don't hide common classes / hide imports #22

Open
ep12 opened this issue Aug 5, 2018 · 7 comments
Open

Don't hide common classes / hide imports #22

ep12 opened this issue Aug 5, 2018 · 7 comments

Comments

@ep12
Copy link

ep12 commented Aug 5, 2018

If trying to create a higher level script with custom classes, it is helpful to have direct access to the common classes of SoundCard, e.g.:

import soundcard as sc
# ...
class AudioChannel(object):
    def play(self, speaker):
        assert isinstance(speaker, sc.Speaker), 'speaker must be a speaker'
        # ...
```
Currently, those classes are hidden and not accessible. However, internals (imports) like cffi, re, os, ... should not be visible to the developer trying out that package using ipython or similar.
@ep12 ep12 changed the title Don't hide common classes Don't hide common classes / hide imports Aug 5, 2018
@bastibe
Copy link
Owner

bastibe commented Aug 5, 2018

I don't quite understand your comment. What exactly are you trying to do, and what do you think SoundCard is doing wrong?

@ep12
Copy link
Author

ep12 commented Aug 6, 2018

It isn't doing anything wrong, but if one wants to explore a module, it is helpful to hide imports that the developer does not need to see. I changed everything except numpy to start with underscores:
os -> _os
time -> _time
And I renamed _Speaker, _Microphone, _Player and _Recorder so that they are usable. Before,

import soundcard as sc
sc._Speaker

raised an AttributeError: the module has no attribute _Speaker.
My fork here contains those changes and some linter eyecandy (visual indentation, some new docstrings) but I'm still working on it. I'll create a pull request when I'm happy with it.

@bastibe
Copy link
Owner

bastibe commented Aug 7, 2018

There's a reason for the underscores on _Speaker, _Microphone, _Player, and _Recorder: They are not meant to be used by users. There is essentially no way for a user to supply them with their required arguments. That's why they are hidden.

As for renaming common imports, do you have a reference for this? I know this practice exists, but I find it profoundly ugly. Could you accomplish a similar goal by specifying an __all__?

@levic
Copy link

levic commented Oct 10, 2019

What exactly are you trying to do

One use case: I was trying to use type annotations for functions that passed around _Microphone and _Speaker objects but I couldn't because they're not exported.

Could you accomplish a similar goal by specifying an __all__?

Yes, __all__ can include identifiers with underscores; the name alone indicates that it's an internal identifier even if it's exported.

The main downside of __all__ is that it's extra work: you need to keep it up to date for every implementation (pulseaudio/coreaudio/mediafoundation) and then again in __init__.py (although the code is identical between them all).

@levic
Copy link

levic commented Oct 10, 2019

For reference, I've been using this:

__all__ = (
	'_Microphone',
	'_Player',
	'_Recorder',
	'_Speaker',
	'all_microphones',
	'all_speakers',
	'default_microphone',
	'default_speaker',
	'get_microphone',
	'get_speaker',
)

(those are the classes that appears to be common, but I've only tested on windows)

@bastibe
Copy link
Owner

bastibe commented Oct 14, 2019

Yes, type annotations are a good reason for accessing these. But then, I think import soundcard._Microphone is a sensible solution to this problem. It's only four imports, after all.

@bastibe
Copy link
Owner

bastibe commented Apr 28, 2020

If anyone cares enough about this to contribute a pull request, I'll be happy to merge it.

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

No branches or pull requests

3 participants