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

Updates to TC tables in Tax-Calculator PR 1917 #846

Closed
wants to merge 5 commits into from

Conversation

hdoupe
Copy link
Collaborator

@hdoupe hdoupe commented Mar 13, 2018

This PR updates to the tables specified in PSLmodels/Tax-Calculator#1917. The update is fairly straight forward. The table labels still need to be updated. For now, they are just the variable names.

However, as it goes with tables, the hard part to updating them is preserving backwards compatibility. We have a framework for re-formatting the tables dynamically when old tables are loaded, but doing this is tricky (see #738, #814). Further, the actual row labels are set in the java script code. These will need to be updated dynamically, and I may need to rely on @andersonfrailey and @GoFroggyRun for assistance on this.

I anticipate this taking somewhere between three and seven days to get right.

screen shot 2018-03-13 at 10 45 54 am

screen shot 2018-03-13 at 10 46 09 am

@MattHJensen
Copy link
Contributor

Could we break backwards compatibility for the tables temporarily and then restore it later? Is there another way to delay the 3-7 day investment until later? I don't think @andersonfrailey has the bandwidth to work on this right now.

@MattHJensen
Copy link
Contributor

How will the meanings of 0-10n, 0-10z, 0-10p be documented from the perspective of a PolicyBrain user?

@hdoupe
Copy link
Collaborator Author

hdoupe commented Mar 13, 2018

@MattHJensen asked

Could we break backwards compatibility for the tables temporarily and then restore it later? Is there another way to delay the 3-7 day investment until later? I don't think @andersonfrailey has the bandwidth to work on this right now.

That's fine with me. Preserving backwards compatibility within our current table building code doesn't seem like a feasible solution in the long run. It seems likely to me that we will need to rethink our approach to this part of TaxBrain in the future.

@hdoupe
Copy link
Collaborator Author

hdoupe commented Mar 13, 2018

@MattHJensen asked

How will the meanings of 0-10n, 0-10z, 0-10p be documented from the perspective of a PolicyBrain user?

@MaxGhenis and @martinholmer have been thinking much more deeply about this than me. I'm interested in their opinion on this question.

@MattHJensen
Copy link
Contributor

That's fine with me. Preserving backwards compatibility within our current table building code doesn't seem like a feasible solution in the long run. It seems likely to me that we will need to rethink our approach to this part of TaxBrain in the future.

Ok, let's do this.

@MaxGhenis
Copy link
Contributor

How will the meanings of 0-10n, 0-10z, 0-10p be documented from the perspective of a PolicyBrain user?

Would an asterisk/footnote work? Or it looks like there's enough width to add descriptions in the labels, e.g. "0-10: <$0", "0-10: $0", "0-10: >$0".

@hdoupe
Copy link
Collaborator Author

hdoupe commented Mar 13, 2018

@MaxGhenis Doing something like "0-10: <$0", "0-10: $0", "0-10: >$0" makes sense to me. Do you think we should specify what we are measuring? Is this referring to expanded income?

@MaxGhenis
Copy link
Contributor

Do you think we should specify what we are measuring? Is this referring to expanded income?

My understanding is that yes it refers to expanded income. Since the chart title refers to expanded income I don't think another note is necessary.

@hdoupe
Copy link
Collaborator Author

hdoupe commented Mar 13, 2018

Sounds good to me. @MattHJensen do you have any objections to doing it this way?

@MattHJensen
Copy link
Contributor

This approach is fine with me, although I find the 0-10: <$0 nomenclature somewhat odd. 0-10 seems to imply a bound at zero, but then we specify that no, there are negatives.

@MaxGhenis
Copy link
Contributor

MaxGhenis commented Mar 14, 2018 via email

@hdoupe
Copy link
Collaborator Author

hdoupe commented Mar 14, 2018

@MattHJensen While the deciles don't actually say anything about whether the income is positive or negative, it does look a little strange to see 0-10: <$0. I don't know of a better way to indicate this besides saying something like Bottom decile: <$0 or 0-10 (decile): <$0.

@MaxGhenis
Copy link
Contributor

Thinking about PSLmodels/Tax-Calculator#1917 (comment) I think there's a symmetry between the -/0/+ split of the bottom decile and the 90-95/95-99/top1% split of the upper decile. And actually I wonder if having the total and then indented break-outs would be clearest for both cases.

As mentioned there, this would require left-aligning the labels instead of center-aligning. In this case it would also involve reporting the full bottom decile (metrics that don't make sense for this group like %change could be nulled out). But it would clarify the labeling and ensure that the TOTAL row equals the sum of the un-indented deciles, avoiding the awkward footnote rows below TOTAL.

The rows would then look like

0-10
  <$0
  $0
  >$0
10-20
20-30
30-40
40-50
50-60
60-70
70-80
80-90
90-100
  90-95
  95-99
  Top 1%
TOTAL

@martinholmer
Copy link
Contributor

@MaxGhenis said in PolicyBrain(TaxBrain) pull request #846:

Thinking about PSLmodels/Tax-Calculator#1917 (comment) I think there's a symmetry between the -/0/+ split of the bottom decile and the 90-95/95-99/top1% split of the upper decile. And actually I wonder if having the total and then indented break-outs would be clearest for both cases.

As mentioned there, this would require left-aligning the labels instead of center-aligning. In this case it would also involve reporting the full bottom decile (metrics that don't make sense for this group like %change could be nulled out). But it would clarify the labeling and ensure that the TOTAL row equals the sum of the un-indented deciles, avoiding the awkward footnote rows below TOTAL.

The rows would then look like

0-10
 <$0
 $0
 >$0
10-20
20-30
30-40
40-50
50-60
60-70
70-80
80-90
90-100
 90-95
 95-99
 Top 1%
TOTAL

I'm totally mystified by this suggestion. @MaxGhenis, it was your complaint (that showing the whole bottom decile in the tables and graphs was misleading) that triggered all the changes we see here. And now you want TaxBrain to show the whole bottom decile, which you previously asserted was misleading. You need to explain your views on this in more detail because a casual reader of the discussions that began late last year might come to the conclusion that you've changed your mind.

As I said earlier, Tax-Calculator is agnostic about the best was to show these tables and graphs, and therefore, leaves it up to each user about how to use the table information generated by Tax-Calculator.

If TaxBrain wants to display the tables the way you now suggest, TaxBrain has all the information it needs to do that. This is a TaxBrain decision, not a Tax-Calculator decision.

@MaxGhenis
Copy link
Contributor

now you want TaxBrain to show the whole bottom decile, which you previously asserted was misleading.

I maintain my assertion that certain metrics are misleading to report for the full bottom decile, particularly % change to after-tax income (the primary metric I look at for distributional analysis), which I suggested nulling out in this table. But summation metrics, as opposed to ratios, can still be informative for the full bottom decile, especially to the extent that people expect the sum of each decile to match the total row, as you mentioned.

@MaxGhenis
Copy link
Contributor

If TaxBrain wants to display the tables the way you now suggest, TaxBrain has all the information it needs to do that. This is a TaxBrain decision, not a Tax-Calculator decision.

Are difference table row labels intended to match between TaxBrain and Tax-Calculator? If so, this issue is relevant for both TaxBrain and Tax-Calculator. Where should it be discussed? It seems like Tax-Calculator is more natural, since TaxBrain could "inherit" the table labels for the specification of the difference table function it calls.

@MattHJensen
Copy link
Contributor

If TaxBrain wants to display the tables the way you now suggest, TaxBrain has all the information it needs to do that. This is a TaxBrain decision, not a Tax-Calculator decision.

I would very much like TaxBrain to display things the same way that Tax-Calculator does, and for these to be Tax-Calculator decisions. (In general, we are trying to move modeling-related decisions to underlying projects so that PolicyBrain developers don't need to maintain domain expertise -- this is one reason, for example, why PolicyBrain is asking OG-USA to move parameter validation from PolicyBrain to OG-USA.) Given that, I probably should have asked my initial question about documenting 0-10n, 0-10z, 0-10p over in Tax-Calculator, and I will move the question there.

@hdoupe
Copy link
Collaborator Author

hdoupe commented Mar 19, 2018

Closed in favor of #848

@hdoupe hdoupe closed this Mar 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants