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

Denver Property Tax Relief #4833

Merged
merged 35 commits into from
Aug 22, 2024
Merged

Conversation

hua7450
Copy link
Collaborator

@hua7450 hua7450 commented Aug 3, 2024

Fixes #4782

@hua7450
Copy link
Collaborator Author

hua7450 commented Aug 6, 2024

This PR assigns the maximum values for households that are eligible for the program, following the methods used in Gary-Community-Ventures/benefits-api#461.

Copy link

codecov bot commented Aug 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.01%. Comparing base (f57467b) to head (98b8185).
Report is 32 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #4833    +/-   ##
========================================
  Coverage   99.00%   99.01%            
========================================
  Files        2536     2555    +19     
  Lines       36845    37133   +288     
  Branches      152      158     +6     
========================================
+ Hits        36479    36767   +288     
  Misses        337      337            
  Partials       29       29            

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@PavelMakarchuk PavelMakarchuk marked this pull request as ready for review August 15, 2024 13:49
income = spm_unit("co_denver_property_tax_relief_income", period)
size = spm_unit("spm_unit_size", period)
ami = spm_unit.household("ami", period)
p_hud = parameters(period).gov.hud.ami_limit
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we refactor the file this logic is from to avoid duplication? maybe create separate variables for each of the limits, and use the moderate one here? especially since also in the renter variable

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I combine those variables into one, like my current version? The downside of my current code is I have to input hud_income_level = MODERATE in test cases.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you refactor the original file? As is this duplicates much of the logic.

Copy link
Contributor

@MaxGhenis MaxGhenis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

final stretch here - we may want to refactor further to have the capped and excess size be their own variables (and making 4 a parameter) to reduce duplication, but this works for now

definition_period = YEAR

def formula(spm_unit, period, parameters):
p_hud = parameters(period).gov.hud.ami_limit
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
p_hud = parameters(period).gov.hud.ami_limit
p = parameters(period).gov.hud.ami_limit

size_exceeding_4 = max_(size - 4, 0)
size_capped_at_4 = min_(size, 4)
return (
size_limit.ESPECIALLY_LOW[size_capped_at_4]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same for the excess and other files - in general please keep the p. in where reasonable to indicate it's a parameter

Suggested change
size_limit.ESPECIALLY_LOW[size_capped_at_4]
p.family_size.ESPECIALLY_LOW[size_capped_at_4]

@MaxGhenis MaxGhenis merged commit c3202e9 into PolicyEngine:master Aug 22, 2024
5 of 6 checks passed
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.

Denver Property Tax Relief
3 participants