Conversation
…raisal-costincome
…on-appraisal-costincome
Cleans-up, Docstringfies Better to_dict, color parser, and post_inits Removes duplicate docstring
…MADA-project/climada_python into feature/option-appraisal-dataclasses
…on-appraisal-costincome
…e/option-appraisal-dataclasses
…on-appraisal-costincome
|
This PR is officially open for review! @luseverin would you be so kind to review the tutorial (to spare @chahank who has to review a lot of my code lately) |
|
Hi @spjuhel, yes I'll be happy to review the tutorial :) |
…on-appraisal-costincome
…e/option-appraisal-dataclasses
…on-appraisal-costincome
…on-appraisal-costincome
This reverts commit 775c621.
…on-appraisal-costincome
There was a problem hiding this comment.
Cell 1:
- Do you need to specify the sign convention? I think it might be confusing as the users anyways input the costs with a positive sign.
- Maybe already define the meaning of the values Y, M, Q for the freq parameter here?
Cell 2:
- comment not matching code (20000 vs 50000 and 12000 vs 8000)
Cell 3:
- "Three methods are available to calculate cash flows:
- I would maybe repeat the parameters instead of the ... for clarity
Cell 5:
- Consider an option to add a row with totals in the dataframe format
Cell 6:
- ... draws a bar chart of the cash flows
Cell 7:
- Is it voluntary that you do not compare the same
init_cost,periodic_costandperiodic_incomebetween the with- and without-growth instances? See the comment on cell 2.
Cell 10:
- Not exactly sure of what is meant with "annualised" but it is probably because I am not familiar with the field :) I think I get the idea though
Cell 11:
- Do we care about the days when we consider the monthly case? What happens if we put 2022-01-15 instead of 2022-01-01 for the date? Maybe you could shortly comment on that (if relevant)
Cell 13:
- Maybe add an example of the manual merge of the custom flows?
Cell 15:
- The From YAML part seems to be incomplete here, but you are probably aware of it :)
luseverin
left a comment
There was a problem hiding this comment.
Hey @spjuhel,
I just reviewed the tutorial, great work! As a new user of the module, with a limited knowledge on the topic I could understand everything, so I think it is a good sign! The module looks very neat and I look forward to experimenting it, congrats! I had only minor comments (see comments on the file). Apologises as I wrote them as a block because I could not find the option to do the line by line (or cell by cell) commenting in the notebook directly.
Changes proposed in this PR:
Introduces
CostIncomeclass to represent the financial aspect of adaptation measures (Measures) (essentially initial and maintenance costs and possible income)Tutorial here: https://climada-python--1277.org.readthedocs.build/en/1277/user-guide/climada_cost_income.html
PR Author Checklist
develop)PR Reviewer Checklist