PolicyEngine / policyengine.dev

Information for PolicyEngine developers on how to contribute and how it's structured.
1 stars 3 forks source link

Describe the `@dataset` decorator in guidelines around adding new datasets #11

Open MaxGhenis opened 2 years ago

MaxGhenis commented 2 years ago

e.g. advantages over inheriting from Dataset

nikhilwoodruff commented 2 years ago

At the time of writing it, my thinking was that using a class decorator would provide more flexibility and allow the dataset classes to be more minimal in terms of boilerplate code. For example, the class decorator checks for the presence of different functions, generate, save, etc. and applies necessary decorators like staticmethod so these don't need to be specified when defining the classes. But there might be a way to do this with inheritance. Inheritance has the added advantage of code hints and IDE understanding.

baogorek commented 2 years ago

As I develop the CE classes, I'll also submit documentation so future developers can get up to speed more quickly. I will say, as a mediocre programmer, these decorators have been challenging to wrap my head around. Documentation will help but onboarding future developers might still be a challenge.

If you want me to take a crack at an alternative design, I can do that as well. I would even see if I could avoid inheritance (owing to the Composition over Inheritance design pattern).

nikhilwoodruff commented 2 years ago

Thanks @baogorek - that'd be really helpful. I can definitely understand the opacity of the class decorator. If you wanted to have a go at an alternative, feel free - happy to review also.

MaxGhenis commented 2 years ago

Thanks @baogorek for the Composition over Inheritance principle - after watching this video on that, I also wonder making Dataset's generate method an abstractmethod could be appropriate.