-
-
Notifications
You must be signed in to change notification settings - Fork 157
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
replace locals() update with global dictionary #10
Milestone
Comments
Closed
This idea makes sense to me too. +1. |
Dealt with in #70 among others. |
Amy-Xu
pushed a commit
to Amy-Xu/Tax-Calculator
that referenced
this issue
Mar 9, 2016
Drop Q calculation for plan Y
jdebacker
pushed a commit
that referenced
this issue
Jun 7, 2023
Update reform file and aggregate files
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I knew from somewhere that we really should not be modifying the locals() dictionary as it's part of Python's scaffolding, but didn't vocally object too much because a) there were more important things on our plate and b) I wasn't 100% sure I was right. I now have conclusive proof to back my opinion up and would like to make sure our code adheres to the standard.
Conceptually, it's an easy fix: we simply use the "y" (or whatever we decide to name it) dictionary with our tax parameter names as keys.
In practice this might take some time since we have to go through the entire script changing all the appropriate variable names to dictionary keys.
In case you're worried about performance issues associated with the additional step of looking up the dictionary name, then the key, I have two things to say:
The text was updated successfully, but these errors were encountered: