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

Update ImpactFuncSet.__init__ #568

Merged
merged 19 commits into from
Nov 18, 2022
Merged

Conversation

peanutfun
Copy link
Member

@peanutfun peanutfun commented Oct 27, 2022

Changes proposed in this PR:

  • Add option to pass an iterable of ImpactFuncs to the ImpactFuncSet constructor
  • Update tests

Note: This is branched off #550, which should be merged before this branch.

PR Author Checklist

PR Reviewer Checklist

@peanutfun peanutfun changed the base branch from main to develop October 27, 2022 12:34
peanutfun and others added 5 commits October 27, 2022 14:55
Remove initialization with set because it does not guarantee insertion
order.
# Conflicts:
#	climada/entity/impact_funcs/impact_func_set.py
#	climada/entity/impact_funcs/test/test_imp_fun_set.py
#	doc/tutorial/climada_engine_Forecast.ipynb
@emanuel-schmid
Copy link
Collaborator

@peanutfun - what needs yet to be done before this is ready to merge?

@peanutfun
Copy link
Member Author

Hm, seems like I missed some occurances of the constructor. Thanks for adding them, @emanuel-schmid! ☺️

I was wondering if we should keep the Tag object as argument or use **tag_kwargs like in case of Hazard (however, in this case it was necessary because haz_type is a tag argument that was already part of the signature). Maybe @chahank can chip in here?

Otherwise, from what I can see, this is ready to go!

@emanuel-schmid
Copy link
Collaborator

Valid point. Though, the whole Tag concept is overdue for a major overhaul. Hence I'd go for the least possible amount of changes and would rather keep it for the time being.

@emanuel-schmid emanuel-schmid marked this pull request as ready for review November 18, 2022 14:10
@emanuel-schmid emanuel-schmid merged commit eb22779 into develop Nov 18, 2022
@emanuel-schmid
Copy link
Collaborator

🙌

@emanuel-schmid emanuel-schmid deleted the feature/init-impact-func-set branch November 18, 2022 16:56
simonameiler pushed a commit that referenced this pull request Aug 24, 2023
* Add all ImpactFunc attributes to ImpactFunc.__init__

* Add type hints to ImpactFunc.__init__

* Update ImpactFunc classmethods to new init

* Update tests to new ImpactFunc.__init__

* Update docs on InputVar regarding ImpactFunc

* Update tutorials with new ImpactFunc.__init__

* Update ImpactFunc.__init__ docstring

* Update argument order of ImpactFunc.__init__

* Fix linter issue

* Update tests to new ImpactFunc.__init__

* Update tutorials with new ImpactFunc.__init__

* Enable passing impact funcs to ImpactFuncSet constructor

* Update code base to new ImpactFuncSet constructor

* Fix unit test of ImpactFuncSet

Remove initialization with set because it does not guarantee insertion
order.

* undo re-plotting

* Incorporate new ImpactFuncSet.__init__ method

Co-authored-by: emanuel-schmid <schmide@ethz.ch>
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

Successfully merging this pull request may close these issues.

2 participants