search

mk-fg / python-pulse-control

Python high-level interface and ctypes-based bindings for PulseAudio (libpulse)
https://pypi.org/project/pulsectl/
MIT License
170 stars 36 forks source link

Impossible inheriting Pulse class #42

Closed timojuez closed 4 years ago

timojuez commented 4 years ago

Hi, thanks for the project. Now the issue: Inherited classes cannot use the functions like sink_input_list(), this seems to depend on some decorator. Would be nice if the following print() calls would output the same.

import pulsectl
p=pulsectl.Pulse()
print(p.sink_input_list())
class X(pulsectl.Pulse): pass
x=X()
print(x.sink_input_list())
mk-fg commented 4 years ago

I agree, it would be kinda nice.

Dunno how to fix it though, and given that this class is not intended for use with inheritance, don't think it's worth looking into, especially given somewhat different behaviors of this system between python2/python3. If there's a simple fix, can probably merge it though.

Would strongly suggest to avoid using inheritance with third-party modules like this one, unless they are explicitly designed for that (some are, this one is obviously not), as it doesn't work well for such use-case - wrappers are almost always a better way to go, but that's just my opinion.

timojuez commented 4 years ago

Thank you. Well, inheritance and my example work if the new class is called 'Pulse'. So I have no problem if this [1] is being replaced by "if True:" [1] https://github.com/mk-fg/python-pulse-control/blob/405211b115c847955e895bd1160d167698dd56d1/pulsectl/pulsectl.py#L535.

Also it's possible to forbid inheritance by implementing a metaclass so that we know. Or fix it when python2 is abandoned ;)

mk-fg commented 4 years ago

Hm, didn't expect it to be as simple as classname check, which I think is a hack to tell whether it's a passed ctypes func or wrapper func Pulse, can probably be fixed...

mk-fg commented 4 years ago

Should be fixed properly in 7d0f78e As far as I can tell that func is not used anywahere as a decorator anyway by now, so no point hauling those ugly hacks around.

Thanks for reporting and patience.

It was totally a dead code bug here, even though I didn't really care about your use-case and dismissed it as generally "not worth the trouble supporting" for bogus reasons (it didn't work not due to method-wrapper weirdness, but due to a garden-variety bug that you pointed out).

timojuez commented 4 years ago

Oh, the code even became a lot less complex. Thank you! :)