victoresque / pytorch-template

PyTorch deep learning projects made easy.
MIT License
4.75k stars 1.09k forks source link

Abstract base classes do not use ABCMeta metaclass #70

Open joshuacwnewton opened 4 years ago

joshuacwnewton commented 4 years ago

First, thank you for taking the time and effort to create this useful template! :)


I noticed for base_model.py and base_trainer.py, you are using the abc.abstractmethod decorator from the abc module. However, as currently implemented, I think this decorator might be used incorrectly, as the abstract base classes do not use abc.ABCMeta. For more information, see https://docs.python.org/3/library/abc.html#abc.abstractmethod

Using this decorator requires that the class’s metaclass is ABCMeta or is derived from it. A class that has a metaclass derived from ABCMeta cannot be instantiated unless all of its abstract methods and properties are overridden.

To demonstrate: If I define a new class derived from BaseTrainer, but I do not override _train_epoch, I will still be allowed to instantiate an object of that new class. But, @abstractmethod is supposed to prevent this. To fix the issue, change BaseTrainer to BaseTrainer(metaclass=ABCMeta). Once this change is made, the instantiation will throw a TypeError as expected in the case that _train_epoch was not overridden.