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

Fixes issue236 by implementing inclusion_override logic #241

Merged
merged 8 commits into from
Sep 11, 2024

Conversation

eriksynn
Copy link
Collaborator

Removed unused default_inclusion_by_calculation logic

Removed deprecated analysis_type_override code

Added code to pass inclusion_override input through to billing_period records

Added logic to modify calculated AnalysisType based on user inclusion_override preference

Updated dummy test data to set inclusion_override default appropriately now that we are actually using the value

Updated test setup code to correctly read inclusion_override value from csv input file

@eriksynn
Copy link
Collaborator Author

Woops. Looks like I forgot to run the static analysis tools prior to creating my PR. I will fix and update.

@eriksynn
Copy link
Collaborator Author

Ok, there is two errors remaining which I could use help resolving:

src\rules_engine\engine.py:49: error: Argument "inclusion_override" to "NormalizedBillingPeriodRecordBase" has incompatible type "bool | None"; expected "bool" [arg-type]
src\rules_engine\engine.py:75: error: Argument "inclusion_override" to "NormalizedBillingPeriodRecordBase" has incompatible type "bool | None"; expected "bool" [arg-type]

@eriksynn
Copy link
Collaborator Author

Oh, and I should probably add a new test for inclusion_override. Thought it is in fact tested in the Breslow test case (and possibly one other).

@eriksynn
Copy link
Collaborator Author

eriksynn commented Aug 28, 2024

This evening we reviewed this PR as a group with Steve Breit. He pointed out differences between our Analysis Type and Inclusion Override implementation and the design of the UI. I opened issue #242 to capture the missing logic around auto-including or auto-excluding (graying-out) cusp winter months (November and April in Massachusetts). What I think I learned during the conversation is the following, when considering the "end date" of an energy bill...

Nov through Apr should be considered "winter months"; AnalysisType is ALLOWED_HEATING_USAGE; user can override to exclude
November and April will typically be grayed out in the UI as cusp "shoulder months", user can override to include these bills
True shoulder months will always be grayed out in the UI; AnalysisType is NOT_ALLOWED...; the user can't override these bills
July through September are "summer months"; AnalysisType is ALLOWED_NON_HEATING_USAGE; user can override to exclude

@eriksynn
Copy link
Collaborator Author

Oh, and I could use some help/suggestion on how to resolve the mypy type checking issue that's causing the mypy build rule to fail. Thanks!

@eriksynn
Copy link
Collaborator Author

I updated this PR to include an implementation of "winter_cusp_month" which supports the auto-graying out of bills ending in April or November, per our conversation with Steve Breit yesterday. Note, this implementation for cusp months is only implemented for Natural Gas fuel. I have a question to Steve about whether we want the same grayed row behavior for oil/propane fuel bills.

@dwindleduck dwindleduck merged commit f810eee into main Sep 11, 2024
8 checks passed
@dwindleduck dwindleduck deleted the eriksynn/issue236 branch September 11, 2024 00:10
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