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

Clarify documentation of filer input variable #2102

Merged
merged 3 commits into from
Nov 8, 2018
Merged

Clarify documentation of filer input variable #2102

merged 3 commits into from
Nov 8, 2018

Conversation

martinholmer
Copy link
Collaborator

@martinholmer martinholmer commented Nov 5, 2018

The name of the input variable, filer, is misleading because some might think a variable with that name is an output variable that indicates whether or not the unit filed an income tax return in the Tax-Calculator simulation. This would appear to be what happened in issue #2099, which was raised by one of our most advanced users.

Do people think we should change this variable name in the taxdata and Tax-Calculator repositories? If so, what should the new variable name be? Something like data_source? Other ideas?

Can people suggest better records_variables.json documentation text than what is proposed in this pull request?

The focus of this pull request has changed as described here and here.

@MattHJensen @andersonfrailey @hdoupe @evtedeschi3

@codecov-io
Copy link

codecov-io commented Nov 5, 2018

Codecov Report

Merging #2102 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2102   +/-   ##
=======================================
  Coverage   99.91%   99.91%           
=======================================
  Files          16       16           
  Lines        3523     3523           
=======================================
  Hits         3520     3520           
  Misses          3        3

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f327f0...fc8b9f1. Read the comment docs.

@martinholmer
Copy link
Collaborator Author

Tax-Calculator pull request #2102 has been changed to eliminate the filer variable from the Records.USABLE_READ_VARS set. This is an interim state of affairs lasting only until taxdata issue 288 is resolved, in which case the renamed and redefined variable will be reintroduced into the Records.USABLE_READ_VARS set.

@martinholmer martinholmer removed the ready label Nov 8, 2018
@martinholmer martinholmer merged commit ad49057 into PSLmodels:master Nov 8, 2018
@martinholmer martinholmer deleted the fix-filer-doc branch November 8, 2018 23:08
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