Restore metaclass functionality on all python versions

[jancespivo]

Hi,
see

pystatsd/statsd/client.py

Line 84 in ace112e

__metaclass__ = abc.ABCMeta

Unfortunately __metaclass__ has no effect in Python 3. The correct way in Python3 is: class StatsClientBase(metaclass=abc.ABCMeta):

The best way how to keep compatibility with the Python 2 is to use six or future.utils. For only this particular purpose I recommend the future.utils. See http://python-future.org/_modules/future/utils.html#with_metaclass

[jsocol]

Honestly, I'm not sure that the ABCMeta metaclass is doing very much. No tests fail on Python 3.x that pass on Python 2.x (there's an issue with test_ipv6_resolution_udp on Travis but that's true on all Python versions except Pypy).

The only thing we're doing with it is ensuring all the necessary abstract methods are implemented—but neither StatsClientBase nor PipelineBase are intended to be extended by userland code, they're purely internal. That means we can solve this multiple ways. I don't particularly want to add a dependency for this on either six or future. The fact that all the tests pass on 3.6 (at least when I run it locally) is kind of enough for me.

I'm going to pull out the metaclass and dependency on abc, and have the unimplemented methods raise NotImplementedError() instead. But in the meantime, I don't actually believe there's any externally-facing behavior that doesn't work on Py3 here. There are some internal guarantees that aren't being backstopped by the ABCMeta infrastructure—but they are being guaranteed by the test suite.

[jancespivo]

I fully understand your attitude.

We've just implemented our custom statsd client compatible with StatsClient. We use the typechecking via mypy so we need to somewhat declare that our client has the same interface as a StatsClient. It can be simply done with StatstBase.register method provided by ABCMeta. The simple inheritance can cause a problems.

Best regards.

[jsocol]

I'm definitely open to re-adding the metaclass if there are other benefits to it—I'd still rather avoid a dependency here but I think we could vendor in (copy/paste) the with_metaclass helper from future.util.

I'm not entirely sure I'm following, though. Why does statsd.client.StatsClient need to implement register()? It seems like your implementation would be the one declaring itself a StatsClient? Is there a snippet or example code you could share?

[jancespivo]

+1 for idea of vendoring the code from future.util

ABCMeta provides the classmethod register (see https://docs.python.org/3/library/abc.html#abc.ABCMeta.register) so no implementation is needed. Actually I think it is only feature which ABCMeta provides at all.

[jsocol]

Sorry @jancespivo—I was on vacation for a little while. So is my understanding correct that:

class MyABC(metaclass=ABCMeta):
    pass

class X:
    pass

MyABC.register(X)

issubclass(X, MyABC)  # is True

Right? While that makes sense for built-in or other 3rd-party classes, I guess I don't understand the types of problems that directly subclassing StatsClient (or StatsClientBase—if you want to venture into private code) would cause?

[jancespivo]

@jsocol Now it is not a problem, it is for the future. For example if the __setattr__ will be implemented in StatsClient or StatsClientBase for some unknown reason, it will influence all inherited classes. The ABCMeta separates the implementation and the interface.

[jsocol]

I’ve updated the title here to give a more accurate (and less alarming) description, but am going to leave closed for right now as there don’t actually appear to be any practical repercussions of the removal right now.

However, like I said, I’m definitely open to re-introducing the metaclass if there’s a demonstrable issue today, or if we introduce one in the future eg with __setattr__. If we do, I want to make sure we have some test of the metaclass functionality within this library’s test suite.