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

Can we have a one-line initialisation of ImpactFuncSet? #342

Closed
ChrisFairless opened this issue Jan 6, 2022 · 4 comments
Closed

Can we have a one-line initialisation of ImpactFuncSet? #342

ChrisFairless opened this issue Jan 6, 2022 · 4 comments
Assignees

Comments

@ChrisFairless
Copy link
Collaborator

When I create an impact function set from some impact function impf I always do it with

impfset = ImpactFuncSet()
impfset.append(impf)

In the spirit of not initialising empty classes, I'd like to either

  • write an ImpactFuncSet.from_impf method which initialises from a list of impact functions
  • modify ImpactFuncSet.__init__ to take a list of impact functions on initialisation

Is this a good idea? Any preferences?

@chahank
Copy link
Member

chahank commented Jan 6, 2022

Great point!

I would argue for making a good __init__. Ideally, all elements of the impact function set should be set at initialization.

@ChrisFairless
Copy link
Collaborator Author

Yeah a good __init__ would be better. But would that mess with CLIMADA's, uh, 'style', where our __init__ methods never take parameters, and instead initialise with from_* methods?

@chahank
Copy link
Member

chahank commented Jan 7, 2022

I agree, but we can try to improve in the future. Some classes already have it.

I would propose to do the __init__ as it does not hurt.

@emanuel-schmid : do you have any feedback here?

@peanutfun
Copy link
Member

One-line initialization of ImpactFuncSet was implemented in #568

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

No branches or pull requests

4 participants