policyengine-us icon indicating copy to clipboard operation
policyengine-us copied to clipboard

Refactor AR income tax to use a ar_low_income_tax_eligible variable

Open xianxinzhu opened this issue 1 year ago • 4 comments

Fixes #3870

xianxinzhu avatar Mar 01 '24 07:03 xianxinzhu

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.15%. Comparing base (fdc3a14) to head (e8d3aa1).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4048   +/-   ##
=======================================
  Coverage   99.15%   99.15%           
=======================================
  Files        2403     2404    +1     
  Lines       34800    34817   +17     
  Branches      164      164           
=======================================
+ Hits        34507    34524   +17     
  Misses        261      261           
  Partials       32       32           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Mar 01 '24 07:03 codecov[bot]

Thanks for updating to 2023.

If we have to run the whole select statement to determine eligibility, it doesn't save any computation. Given this adds duplicate code, what benefit does this provide? Note I suggested in the issue comparing to the last scale parameter threshold rather than running the full computation, though even that may not be worth it if it adds significant complexity. @PavelMakarchuk do you recall the context of me filing the issue?

I believe it was to remove the .inf values from the parameter files as it was not how the tax forms represent this logic - we would rather have an eligibility variable

PavelMakarchuk avatar Mar 22 '24 15:03 PavelMakarchuk

Ok can we do that here then?

MaxGhenis avatar Mar 22 '24 16:03 MaxGhenis

Ok can we do that here then?

Not sure how we would do it, I think our thinking was to generate an eligibility file to avoid the output to return .inf values

PavelMakarchuk avatar Mar 29 '24 15:03 PavelMakarchuk