You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
The text was updated successfully, but these errors were encountered:
SunQpark
added a commit
to SunQpark/pytorch-template
that referenced
this issue
Jun 12, 2020
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 theabc
module. However, as currently implemented, I think this decorator might be used incorrectly, as the abstract base classes do not useabc.ABCMeta
. For more information, see https://docs.python.org/3/library/abc.html#abc.abstractmethodTo 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, changeBaseTrainer
toBaseTrainer(metaclass=ABCMeta)
. Once this change is made, the instantiation will throw a TypeError as expected in the case that_train_epoch
was not overridden.The text was updated successfully, but these errors were encountered: