From 4e4c71a07c79ec57a60d2bff87ad054314f115cb Mon Sep 17 00:00:00 2001 From: Jacob C <44846403+chusloj@users.noreply.github.com> Date: Mon, 28 Dec 2020 16:30:49 -0500 Subject: [PATCH 1/7] check for existence of reform/assumption --- taxcalc/calculator.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/taxcalc/calculator.py b/taxcalc/calculator.py index 4605f134f..e84eccdb3 100644 --- a/taxcalc/calculator.py +++ b/taxcalc/calculator.py @@ -8,6 +8,7 @@ # pylint: disable=too-many-lines,no-value-for-parameter import copy +import os import numpy as np import pandas as pd import paramtools @@ -1086,6 +1087,11 @@ def read_json_param_objects(reform, assump): The 'growdiff_response' subdictionary of the returned dictionary is suitable as input into the GrowDiff.update_growdiff method. """ + # check for existence of reform + if not os.path.isfile(reform): + raise ValueError("This .json reform file does not exist or this is a misspecified directory.") + if not os.path.isfile(assump): + raise ValueError("This .json assumptions file does not exist or this is a misspecified directory.") # construct the composite dictionary param_dict = dict() param_dict['policy'] = Policy.read_json_reform(reform) From 3763868c93348a7bf7c951989c78efafa412bf87 Mon Sep 17 00:00:00 2001 From: Jacob C <44846403+chusloj@users.noreply.github.com> Date: Mon, 28 Dec 2020 16:33:56 -0500 Subject: [PATCH 2/7] change comment --- taxcalc/calculator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taxcalc/calculator.py b/taxcalc/calculator.py index e84eccdb3..c3b2750ca 100644 --- a/taxcalc/calculator.py +++ b/taxcalc/calculator.py @@ -1087,7 +1087,7 @@ def read_json_param_objects(reform, assump): The 'growdiff_response' subdictionary of the returned dictionary is suitable as input into the GrowDiff.update_growdiff method. """ - # check for existence of reform + # check for existence of reform and assumption files if not os.path.isfile(reform): raise ValueError("This .json reform file does not exist or this is a misspecified directory.") if not os.path.isfile(assump): From 5b0d54a240c6aecfc580b5c0e61a3ecc6a7ce830 Mon Sep 17 00:00:00 2001 From: Jacob C <44846403+chusloj@users.noreply.github.com> Date: Tue, 29 Dec 2020 12:39:41 -0500 Subject: [PATCH 3/7] check reform files for type and contents --- taxcalc/calculator.py | 7 ------- taxcalc/parameters.py | 12 +++++++++++- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/taxcalc/calculator.py b/taxcalc/calculator.py index c3b2750ca..875a3bcee 100644 --- a/taxcalc/calculator.py +++ b/taxcalc/calculator.py @@ -8,7 +8,6 @@ # pylint: disable=too-many-lines,no-value-for-parameter import copy -import os import numpy as np import pandas as pd import paramtools @@ -1087,12 +1086,6 @@ def read_json_param_objects(reform, assump): The 'growdiff_response' subdictionary of the returned dictionary is suitable as input into the GrowDiff.update_growdiff method. """ - # check for existence of reform and assumption files - if not os.path.isfile(reform): - raise ValueError("This .json reform file does not exist or this is a misspecified directory.") - if not os.path.isfile(assump): - raise ValueError("This .json assumptions file does not exist or this is a misspecified directory.") - # construct the composite dictionary param_dict = dict() param_dict['policy'] = Policy.read_json_reform(reform) param_dict['consumption'] = Consumption.read_json_update(assump) diff --git a/taxcalc/parameters.py b/taxcalc/parameters.py index b0408a164..b79387dda 100644 --- a/taxcalc/parameters.py +++ b/taxcalc/parameters.py @@ -692,7 +692,17 @@ def convert_year_to_int(syr_dict): req.raise_for_status() txt = req.text else: - txt = obj + if isinstance(topkey, str) and (topkey == ''): + raise ValueError("String is empty.") + if isinstance(obj, str): + if obj == '': + raise ValueError("String is empty.") + elif ("{" or "}") in obj: + txt = obj + else: + raise ValueError("The .json file or variable does not exist or is misspecified.") + else: + raise ValueError("The .json file or variable does not exist or is misspecified.") # strip out //-comments without changing line numbers json_txt = re.sub('//.*', ' ', txt) # convert JSON text into a Python dictionary From 0aec1c944d01d0132169b400560cd9951f61ef5e Mon Sep 17 00:00:00 2001 From: Jacob C <44846403+chusloj@users.noreply.github.com> Date: Tue, 29 Dec 2020 12:46:03 -0500 Subject: [PATCH 4/7] rewrite missing line in calculator class --- taxcalc/calculator.py | 1 + 1 file changed, 1 insertion(+) diff --git a/taxcalc/calculator.py b/taxcalc/calculator.py index 875a3bcee..4605f134f 100644 --- a/taxcalc/calculator.py +++ b/taxcalc/calculator.py @@ -1086,6 +1086,7 @@ def read_json_param_objects(reform, assump): The 'growdiff_response' subdictionary of the returned dictionary is suitable as input into the GrowDiff.update_growdiff method. """ + # construct the composite dictionary param_dict = dict() param_dict['policy'] = Policy.read_json_reform(reform) param_dict['consumption'] = Consumption.read_json_update(assump) From 5832d1912e7e29f6adf1472108e7e48ba54b426c Mon Sep 17 00:00:00 2001 From: Jacob C <44846403+chusloj@users.noreply.github.com> Date: Tue, 29 Dec 2020 19:53:10 -0500 Subject: [PATCH 5/7] 2 curly braces form a json statement --- taxcalc/parameters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taxcalc/parameters.py b/taxcalc/parameters.py index b79387dda..b3d5111a7 100644 --- a/taxcalc/parameters.py +++ b/taxcalc/parameters.py @@ -697,7 +697,7 @@ def convert_year_to_int(syr_dict): if isinstance(obj, str): if obj == '': raise ValueError("String is empty.") - elif ("{" or "}") in obj: + elif ("{" and "}") in obj: txt = obj else: raise ValueError("The .json file or variable does not exist or is misspecified.") From bf71baaba7dc647de74e1d1c74f12a4118f00f68 Mon Sep 17 00:00:00 2001 From: Jacob C <44846403+chusloj@users.noreply.github.com> Date: Mon, 4 Jan 2021 14:15:03 -0500 Subject: [PATCH 6/7] test for non-existent json files --- taxcalc/parameters.py | 4 ++-- taxcalc/tests/test_calculator.py | 12 +++++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/taxcalc/parameters.py b/taxcalc/parameters.py index b3d5111a7..0db10d96a 100644 --- a/taxcalc/parameters.py +++ b/taxcalc/parameters.py @@ -700,9 +700,9 @@ def convert_year_to_int(syr_dict): elif ("{" and "}") in obj: txt = obj else: - raise ValueError("The .json file or variable does not exist or is misspecified.") + raise OSError("The .json file or variable does not exist or is misspecified.") else: - raise ValueError("The .json file or variable does not exist or is misspecified.") + raise OSError("The .json file or variable does not exist or is misspecified.") # strip out //-comments without changing line numbers json_txt = re.sub('//.*', ' ', txt) # convert JSON text into a Python dictionary diff --git a/taxcalc/tests/test_calculator.py b/taxcalc/tests/test_calculator.py index 62c4ba163..b6d822a50 100644 --- a/taxcalc/tests/test_calculator.py +++ b/taxcalc/tests/test_calculator.py @@ -504,12 +504,22 @@ def test_read_bad_json_assump_file(): """ with pytest.raises(ValueError): Calculator.read_json_param_objects(None, badassump1) - with pytest.raises(ValueError): + with pytest.raises(OSError): Calculator.read_json_param_objects(None, 'unknown_file_name') with pytest.raises(ValueError): Calculator.read_json_param_objects(None, list()) +def test_json_doesnt_exist(): + """ + Test JSON file which doesn't exist + """ + with pytest.raises(OSError): + Calculator.read_json_param_objects(None, './reforms/doesnt_exist.json') + with pytest.raises(OSError): + Calculator.read_json_param_objects('./reforms/doesnt_exist.json', None) + + def test_calc_all(): """ Test calc_all method. From 119a946492e7a3233d439176cc34cdf0ad16dc3f Mon Sep 17 00:00:00 2001 From: Jacob C <44846403+chusloj@users.noreply.github.com> Date: Tue, 5 Jan 2021 10:15:14 -0500 Subject: [PATCH 7/7] new error messages for missing file and incorrect JSON formatting --- taxcalc/parameters.py | 10 ++++++---- taxcalc/tests/test_calculator.py | 6 +++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/taxcalc/parameters.py b/taxcalc/parameters.py index 0db10d96a..fc4f919d7 100644 --- a/taxcalc/parameters.py +++ b/taxcalc/parameters.py @@ -693,16 +693,18 @@ def convert_year_to_int(syr_dict): txt = req.text else: if isinstance(topkey, str) and (topkey == ''): - raise ValueError("String is empty.") + raise ValueError("topkey string is empty.") if isinstance(obj, str): if obj == '': - raise ValueError("String is empty.") + raise ValueError("obj string is empty.") + elif obj.endswith('.json') and not os.path.isfile(obj): + raise FileNotFoundError("The .json file does not exist.") elif ("{" and "}") in obj: txt = obj else: - raise OSError("The .json file or variable does not exist or is misspecified.") + raise ValueError("The JSON variable is misspecified.") else: - raise OSError("The .json file or variable does not exist or is misspecified.") + raise ValueError("The JSON variable is misspecified.") # strip out //-comments without changing line numbers json_txt = re.sub('//.*', ' ', txt) # convert JSON text into a Python dictionary diff --git a/taxcalc/tests/test_calculator.py b/taxcalc/tests/test_calculator.py index b6d822a50..53d7e0610 100644 --- a/taxcalc/tests/test_calculator.py +++ b/taxcalc/tests/test_calculator.py @@ -504,7 +504,7 @@ def test_read_bad_json_assump_file(): """ with pytest.raises(ValueError): Calculator.read_json_param_objects(None, badassump1) - with pytest.raises(OSError): + with pytest.raises(ValueError): Calculator.read_json_param_objects(None, 'unknown_file_name') with pytest.raises(ValueError): Calculator.read_json_param_objects(None, list()) @@ -514,9 +514,9 @@ def test_json_doesnt_exist(): """ Test JSON file which doesn't exist """ - with pytest.raises(OSError): + with pytest.raises(FileNotFoundError): Calculator.read_json_param_objects(None, './reforms/doesnt_exist.json') - with pytest.raises(OSError): + with pytest.raises(FileNotFoundError): Calculator.read_json_param_objects('./reforms/doesnt_exist.json', None)