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

Processor ancestor class and ABCs #36

Open
fish2000 opened this issue Jan 17, 2019 · 2 comments
Open

Processor ancestor class and ABCs #36

fish2000 opened this issue Jan 17, 2019 · 2 comments

Comments

@fish2000
Copy link
Contributor

Hi, I’m Alexander Böhn, longtime fan of django-imagekit, PILKit, and the maintainer of Instakit, here with a question that is relevant to both your project and mine.

One thing I have consistently enjoyed about programming processors – either for use with django-instakit or for other purposes – is the fact that they are so simple: the requirement is merely a class with a process(self, image) method.

Lately, though, I have been stretching the limit of this otherwise elastic design feature: I am implementing an automatically-generated CLI using the argparser module, to afford an Instakit user a sort of imaging testing gallery.

python

To do this, I am employing a bunch of honestly rather gnarly heuristic assumptive decisions in the code - stuff that could be seriously stabilized by using an ABC, e.g. like:

import abc

class ProcessorBase(abc.ABC):

    @abc.abstractmethod
    def process(self, image): ...

And so while I am loathe to complicate what I consider to be one of the major selling points of PILKit and its ilk (that being, the lack of hand-wavey code theater necessary to start working) it has not escaped my attention that using some sort of common ancestor of this sort could be a good thing. Furthermore, the use of such a thing could even be optional – imagine for a moment, with me, a world in which a processor is still any class adopting the process(self, image) method (which in Objective-C-land we call an “informal protocol”) but one can also opt-in to using an ABC ancestor along the lines of the one above.

What do you think?

@vstoykov
Copy link
Collaborator

Sounds very logical. The Python follows Duck Typing and requiring the user to always extends some class just to overwrite everything in it is not very good. Using these abstract classes is more from static typed languages and there is literally no benefit at runtime (even there can be a slowdown) when you are using them but they can benefit IDEs or some other tools that will inspect the code.

Because it can be optional and will not break anything I'll accept PR with proposed changes but unfortunately I don't have enough spare time to prepare it myself. If you are willing to prepare a PR I can happily accept it.

@fish2000
Copy link
Contributor Author

@vstoykov Absolutely I would do it, with tests and all. Let me get to work on it when I have a little time to spare and I’ll push it up when it’s ready for review.

Although to your point about runtime advantage: using ABCs allows you to easily enumerate the subclasses of a class that is descendant from an ABC, including so-called “virtual subclasses” – the use-case for which is that third-party module writers can then non-invasively declare things to be subclasses of an original modules’ ABC descendant, without worrying about breaking anything or having to fork or vendor the module code.

It’s these mechanics that I am interested in – and fortunately your point about the lack of runtime penalties is correct! I was skeptical at first, but in my last few projects I have made extensive use of ABCs in one way or another and they have always been a boon, never an albatross.

Thank you sir!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants