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

Adds an output year parameter #789

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

hdoupe
Copy link
Collaborator

@hdoupe hdoupe commented Dec 22, 2017

Resolves #763. This PR adds an output start year parameter.

@hdoupe
Copy link
Collaborator Author

hdoupe commented Dec 22, 2017

@GoFroggyRun Would you mind taking a look at this? I tried to copy the same code that was used for the start_year parameter. I get the new drop down menu:
screen shot 2017-12-22 at 11 49 08 am

However, nothing happens when I select a new year.

@hdoupe hdoupe added the WIP label Jan 4, 2018
@GoFroggyRun
Copy link
Contributor

GoFroggyRun commented Jan 5, 2018

@hdoupe I'm working on this, and might have some ways to deal with it. However, my local run hangs on "a few seconds remaining", even on current master. Do we have systematic ways to resolve such local issue?

@hdoupe
Copy link
Collaborator Author

hdoupe commented Jan 5, 2018

Great thanks @GoFroggyRun. The issue is that the time per run is calibrated for the AWS machines. They seem to run a year of calculations at around 30 seconds or less while I get times closer to 60-70 seconds locally. The hanging results because TaxBrain thinks the calculations should be ready but they are actually still finishing up. This can be verified by checking the output of the log files in deploy/taxbrain-server/logs/celery-stderr---supervisor...log

This can be a little annoying but since it doesn't cause any problems in production I just ignore it.

@GoFroggyRun
Copy link
Contributor

@hdoupe, after playing with TaxBrain a bit, I actually don't see an easy way to separate "start year" and "output year", where TaxBrain seems to assume the two to be always the same. The only way to modify output year that I am aware of is to change the start_year value, which is associated with the year of input form (i.e. default year of parameters). Adding a second parameter won't do anything unless it deals with start_year, which in turn controls both output year and default input year.

Are there, theoretically, any ways to have TaxBrain in, say, year 2017 on the input page while in year 2018 for output pages?

@hdoupe
Copy link
Collaborator Author

hdoupe commented Jan 8, 2018

@GoFroggyRun asked

Are there, theoretically, any ways to have TaxBrain in, say, year 2017 on the input page while in year 2018 for output pages?

I think so. It will take a little bit more work on the back-end once the front-end is set up to send and receive an output start year. One approach is to use the input start year to create the reform here and the output start year when you submit the reform here. That way the reform is specified with the input year and the Tax-Calculator start year is the output start year.

@GoFroggyRun
Copy link
Contributor

Cool. Thanks for your explanation. If this is the case, then we shouldn't be surprised at

However, nothing happens when I select a new year.

Right? We won't see anything happen until back-end has something similar to start_year that indicates the output year.

Would it be more reasonable to have a variable added on backend first so that we can fully test functionalities more adequately while working on the potential output year dropdown button?

@hdoupe
Copy link
Collaborator Author

hdoupe commented Jan 8, 2018

@GoFroggyRun When I first tried to put this PR together, I couldn't get anything to happen when I changed the year. However, I just checked out the PR and got the following result:

screen shot 2018-01-08 at 3 54 25 pm

Previously, nothing happened when I clicked a different start year in the drop down menu. I'm not completely sure why it's working now and it didn't before.

Thanks for reviewing. I'll keep working on this PR and let you know if I have any more problems.

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.

2 participants