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

Abstract base classes do not use ABCMeta metaclass #70

Open
joshuacwnewton opened this issue Apr 4, 2020 · 0 comments
Open

Abstract base classes do not use ABCMeta metaclass #70

joshuacwnewton opened this issue Apr 4, 2020 · 0 comments
Labels
good first issue Good for newcomers

Comments

@joshuacwnewton
Copy link

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.

SunQpark added a commit to SunQpark/pytorch-template that referenced this issue Jun 12, 2020
fix issue victoresque#70 from victoresque/pytorch-template
@SunQpark SunQpark added good first issue Good for newcomers invalid This doesn't seem right and removed invalid This doesn't seem right labels Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants