-
Notifications
You must be signed in to change notification settings - Fork 28
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
the Gridder class should be an abstract class #45
Comments
there is a similar issue with |
Hi Dominik. If I understood that correctly there is no way to render a class pure abstract in Python as long as you want to support both, Python 2.7 and Python 3.X. In Python 3.X ABC would be the right way to go. |
I will investigate to which level the abc is usable in python 2.7. I think the goal of this is not to make this 100% clean from programming point of view, but to deliver a reasonable error message to the user when he wants to initialize one of these classes which are not fully functional... |
making Gridder an abstract class by requiring all derived classes to override its call method.
@eugenwintersberger could you comment on the mentioned commit? from my point of view thats a viable solution for this issue. if you agree i will put a similar code into the other mentioned classes. the python2 & 3 compatible code was taken from https://stackoverflow.com/questions/35673474/using-abc-abcmeta-in-a-way-it-is-compatible-both-with-python-2-7-and-python-3-5 |
LayerModel, Material, and DarwinModelAlloy are now abstract classes
I consider this issue solved for the moment. Once only python3.3+ is considered as target version one could make improvements and use new functionality of the abc module. |
The
Gridder
class serves as abstraction of all the derived classes Gridder1D, Gridder2D, ...The class itself however should not be usable since its methods rely on properties which are defined only in the derived classes.
@eugenwintersberger what is the best/pythonic way to solve this?
I see two options:
option 2 seems easy, but does not look like the best solution...
The text was updated successfully, but these errors were encountered: