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

Update to taxcalc 0.13.0+ #738

Merged
merged 73 commits into from
Nov 20, 2017
Merged

Update to taxcalc 0.13.0+ #738

merged 73 commits into from
Nov 20, 2017

Conversation

hdoupe
Copy link
Collaborator

@hdoupe hdoupe commented Nov 14, 2017

This PR resolves #733 and #735.

TODO:

  • Miscellaneous variable renaming
  • Front-end API changes
  • Back-end API changes (flask_server.py/celery_tasks.py focused changes)

@hdoupe
Copy link
Collaborator Author

hdoupe commented Nov 17, 2017

@GoFroggyRun I've been going through the taxbrain-tablebuilder.js script. I'm having trouble figuring out where it pulls the table names from the tables key in the context.

My question is what happens when I click one of these buttons:
screen shot 2017-11-17 at 5 59 11 pm

The table has keys:

 dict_keys(['result_years', 'tooltips', 'dist1_xbin', 'fiscal_currentlaw', 'diff_ptax_xdec', 'fiscal_change', 'diff_comb_xbin', 'dist2_xdec', 'dist1_xdec', 'diff_comb_xdec', 'diff_ptax_xbin', 'dist2_xbin', 'diff_itax_xdec', 'diff_itax_xbin', 'fiscal_reform'])

How does the javascipt know the name of the table that it wants? How does it know to pull the diff_itax_xbin label from this dictionary when the user clicks Income Tax? I think our problem can be solved if we update this mapping.

@hdoupe
Copy link
Collaborator Author

hdoupe commented Nov 17, 2017

I think our problem can be solved if we update this mapping.

I can confirm that this is the case. I swapped one of the new names back to the old name and that table loaded while the other tables still did not.

@hdoupe
Copy link
Collaborator Author

hdoupe commented Nov 19, 2017

@martinholmer PR PSLmodels/Tax-Calculator#1677 resolved the GDP elasticity table issue. Thanks for the good work.

@GoFroggyRun
Copy link
Contributor

@hdoupe asked:

How does the javascipt know the name of the table that it wants? How does it know to pull the diff_itax_xbin label from this dictionary when the user clicks Income Tax? I think our problem can be solved if we update this mapping.

I believe this is handled here in the taxbrain-tablebuilder.js file. The event section corresponds to the manipulations you described.

@hdoupe
Copy link
Collaborator Author

hdoupe commented Nov 20, 2017

The most recent commit fixes the problem with the tables not showing up. This is a temporary solution that needs to be replaced with a real solution. However, I think this involves having some knowledge of java script and front-end engineering, which I do not have.

@hdoupe
Copy link
Collaborator Author

hdoupe commented Nov 20, 2017

@GoFroggyRun thanks for finding that. But, I'm unable to find the place where it selects the table JSON object and then selects a key such as mY_dec under the old naming scheme or dist2_xdec under the new naming scheme. If we can find that place and swap out the table names, then this would be a better solution than what I did in this commit.

@hdoupe
Copy link
Collaborator Author

hdoupe commented Nov 20, 2017

The only remaining problem (that I know of) is that the sections containing new parameter names are not collapsed when you go to the input page with all default parameters.

@GoFroggyRun Do you know where the collapse/expand logic is and how to fix that?

I would say that this is a more pressing issue than the table mapping issue, given the time constraint and that we have a working solution to the table mapping issue.

@GoFroggyRun
Copy link
Contributor

@hdoupe said:

But, I'm unable to find the place where it selects the table JSON object and then selects a key such as mY_dec under the old naming scheme or dist2_xdec under the new naming scheme.

Does this look like something helpful?

@GoFroggyRun
Copy link
Contributor

Do you know where the collapse/expand logic is and how to fix that?

Not really. Never worked on it before. I will checkout your PR and take a look.

@hdoupe
Copy link
Collaborator Author

hdoupe commented Nov 20, 2017

@GoFroggyRun that was updated with the new table names in this commit

@hdoupe
Copy link
Collaborator Author

hdoupe commented Nov 20, 2017

@GoFroggyRun said

Not really. Never worked on it before. I will checkout your PR and take a look.

Great, thanks.

@hdoupe
Copy link
Collaborator Author

hdoupe commented Nov 20, 2017

@hayleefay do you have any ideas on how to solve either of these front-end issues (the table mapping isssue and the collapse/expand issue)?

@hdoupe
Copy link
Collaborator Author

hdoupe commented Nov 20, 2017

@brittainhard do you have a minute to take a look at the collapse/expand bug that we are trying to solve here?

cc @atxsder

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.

3 participants