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

Move taxdata to using only Python 3.6 #283

Merged
merged 2 commits into from
Sep 11, 2018
Merged

Move taxdata to using only Python 3.6 #283

merged 2 commits into from
Sep 11, 2018

Conversation

martinholmer
Copy link
Contributor

@martinholmer martinholmer commented Sep 10, 2018

This pull request changes the environment.yml specification of the taxdata-dev environment to require Python 3.6.5 or higher. This means Python 2.7 can no longer by used to develop taxdata. This change has been planned for many months. Also, the .travis.yml file has been revised so that the GitHub tests use Python 3.6.

When using Python 3.6.5 and after touching all the *py files in the taxdata repository, I executed "make all". The "make all" ran for five hours and produced the same CSV data files as on the master branch except for one: the puf_data/puf.csv file. The unweighted sum of the e19200 variable changed by one (increasing from 1951888779 to 1951888780), suggesting a float-to-integer rounding difference in one case as documented here. Here is the information on the new and old (master branch) puf.csv files:

taxdata mrh$ ls -l puf_data/puf*csv
-rw-r--r--  1 mrh  staff  56415698 Sep  5 18:02 puf_data/puf-master.csv
-rw-r--r--  1 mrh  staff  56415698 Sep  9 17:43 puf_data/puf.csv
taxdata mrh$ md5 puf_data/puf*csv
MD5 (puf_data/puf-master.csv) = a10091a770472254c50f8985d8839162
MD5 (puf_data/puf.csv) = 3f1c7c2b16b6394a9148779db992bed1

Note that using this new puf.csv file (rather than the master-branch puf.csv file) on the develop-with-python36 Tax-Calculator branch (that is, with the changes in Tax-Calculator pull request 2058) cause no additional changes in pytest results.

@andersonfrailey, can you generate this slightly different puf.csv file on your computer using the changes in this pull request?

@andersonfrailey
Copy link
Collaborator

@martinholmer I will run everything overnight and see if I get the slightly different PUF. I will report back in the morning.

@andersonfrailey
Copy link
Collaborator

@martinholmer after running all of the creation scripts with the new environment specs I produced a PUF with the same MD5 as you did.

@martinholmer
Copy link
Contributor Author

@andersonfrailey said:

after running all of the creation scripts with the new environment specs [in PR #283] I produced a PUF with the same MD5 as you did.

@andersonfrailey, Thanks for double checking.
Shall I merge #283 now?
Do you think we need to issue a new puf.csv even though the Tax-Calculator test results are no different? To be absolutely correct about this, I guess we should issue a new puf.csv file. Is it too much work from your point of view?

@andersonfrailey
Copy link
Collaborator

@martinholmer I'm good with merging.

Issuing a new PUF isn't a lot of work, but Tax-Calculator users may be getting a little overwhelmed with the number of new PUFs we've issued in the past few weeks. If there are no PUF updates coming in the immediate future, lets go ahead and issue a new file just so that everyone is on the same page. If there are (I have no plans of making any, but you might) lets wait until we make those before issuing a new PUF.

@martinholmer
Copy link
Contributor Author

Pull request #283 caused a slight change in the generated puf.csv file. Comparing the new puf.csv file with the old puf-master.csv file, we see there is just one filing unit with a difference of one in the e19200 value:

iMac:puf_data mrh$ ls -l puf*csv
-rw-r--r--  1 mrh  staff  56415698 Sep  5 18:02 puf-master.csv
-rw-r--r--  1 mrh  staff  56415698 Sep  9 17:43 puf.csv

iMac:puf_data mrh$ md5 puf*csv
MD5 (puf-master.csv) = a10091a770472254c50f8985d8839162
MD5 (puf.csv) = 3f1c7c2b16b6394a9148779db992bed1

iMac:puf_data mrh$ diff puf.csv puf-master.csv 
211688c211688
< 0,61,67,1,2,0,0,0,6,0,0,2011,0,1,2,0,0,3,0,820,0,10,10,0,0,-29990000,0,221300,0,0,0,-22000,-251400,0,0,0,0,0,4000,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,7599,0,20,3980,0,57800,0,0,0,0,0,0,0,0,0,211687,872,1,1240000,0,0,-29990000,0,-251400,0,0,0,0,0,0,0
---
> 0,61,67,1,2,0,0,0,6,0,0,2011,0,1,2,0,0,3,0,820,0,10,10,0,0,-29990000,0,221300,0,0,0,-22000,-251400,0,0,0,0,0,4000,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,7598,0,20,3980,0,57800,0,0,0,0,0,0,0,0,0,211687,872,1,1240000,0,0,-29990000,0,-251400,0,0,0,0,0,0,0

iMac:puf_data mrh$ ../../Tax-Calculator/csv_vars.sh puf.csv | grep RECID
74 RECID

iMac:puf_data mrh$ ../../Tax-Calculator/csv_show.sh puf.csv 211687
2 age_head 61
3 age_spouse 67
4 n1820 1
5 n21 2
9 fips 6
12 FLPDYR 2011
14 f6251 1
15 MARS 2
18 XTOT 3
20 e00300 820
22 e00600 10
23 e00650 10
26 e00900 -29990000
28 e01200 221300
32 e02000 -22000
33 e02100 -251400
39 e03230 4000
59 e19200 7599
61 e20100 20
62 e20400 3980
64 p23250 57800
74 RECID 211687
75 s006 872
76 filer 1
77 cmbtp 1240000
80 e00900p -29990000
82 e02100p -251400

iMac:puf_data mrh$ ../../Tax-Calculator/csv_show.sh puf.csv 211687 > new

iMac:puf_data mrh$ ../../Tax-Calculator/csv_show.sh puf-master.csv 211687 > old

iMac:puf_data mrh$ diff new old
18c18
< 59 e19200 7599
---
> 59 e19200 7598

@martinholmer
Copy link
Contributor Author

@andersonfrailey said:

Issuing a new PUF isn't a lot of work, but Tax-Calculator users may be getting a little overwhelmed with the number of new PUFs we've issued in the past few weeks. If there are no PUF updates coming in the immediate future, lets go ahead and issue a new file just so that everyone is on the same page. If there are (I have no plans of making any, but you might) lets wait until we make those before issuing a new PUF.

I don't have any plans to change the puf.csv file contents.

@martinholmer martinholmer merged commit 69b35c7 into PSLmodels:master Sep 11, 2018
@andersonfrailey
Copy link
Collaborator

@martinholmer ok. I'll send out a new PUF later today.

@martinholmer martinholmer deleted the use-python36 branch September 11, 2018 16:13
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