search

codeforboston / home-energy-analysis-tool

https://www.codeforboston.org/projects/
MIT License
8 stars 25 forks source link

Suggested logic changes around Inclusion and AnalysisType #155

Open dwindleduck opened 3 months ago

dwindleduck commented 3 months ago

Added comments and sudo code with suggestions for changes to the AnalysisType model, BillingPeriod class, and the _initialize_billing_periods function. Any changes we decide on will have downstream effects, so I thought we should discuss the logic changes before implementing any code changes.

stevebreit commented 3 months ago

I'm not sure how to formally add my review so perhaps this comment will suffice. I joined the Rules Engine group during the March 12 hackathon. We reviewed the relevant code (not sure which file) and the changes in variable names and logic looked good to me. During the March 19 hackathon, I suggested that for the MVP (beta 1) release, we should use the same logic for deciding allowed usage and default inclusion as the the Excel file, to make verification of the code vs. Excel easier. I have created a Google Sheet that lists the billing-period parameters by month number and suggest implementing this for the MVP.

dwindleduck commented 3 months ago

Thanks Steve, this all makes sense to me. The rules-engine team will postpone implementing any deeper hdd calculations and will add the new logic around inclusion using your google sheet. I actually cannot make it this week, I hope it goes well!

Andrew

On Tue, Mar 26, 2024 at 3:53 PM Steve Breit @.***> wrote:

@.**** requested changes on this pull request.

I inserted two comments in the code. Good to go with my suggested changes.

— Reply to this email directly, view it on GitHub https://github.com/codeforboston/home-energy-analysis-tool/pull/155#pullrequestreview-1961571471, or unsubscribe https://github.com/notifications/unsubscribe-auth/A4NUGVEHY2FMBYOSK6FO6K3Y2HG2JAVCNFSM6AAAAABE3YB2T6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSNRRGU3TCNBXGE . You are receiving this because you authored the thread.Message ID: @.*** .com>