Add sleep commands to avoid HTTP 429 error

[jdebacker]

This PR adds sleep commands to avoid HTTP 429 error from accessing the UN data portal for demographics data.

[jdebacker]

Apparently the format of the JSON files from the UN have changed. Using json.loads works better then the requests.response.json at parsing the JSONs, but errors are still occurring after get multiple pages into the results.

cc @rickecon

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 66.66667% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 79.84%. Comparing base (ff2eb25) to head (3d638e8).
Report is 2 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #906      +/-   ##
==========================================
- Coverage   80.72%   79.84%   -0.88%     
==========================================
  Files          19       19              
  Lines        4499     4506       +7     
==========================================
- Hits         3632     3598      -34     
- Misses        867      908      +41     

Flag	Coverage Δ
unittests	79.84% <66.66%> (-0.88%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
ogcore/__init__.py	100.00% <100.00%> (ø)
ogcore/demographics.py	71.56% <64.70%> (-12.75%)	⬇️

[jdebacker]

@SeaCelo asked a good question -- why download in JSON format as opposed to CSV?

That solved it -- no more "too many requests" because you get in one page of CSV data what 15+ pages of JSON was giving you.

This PR now makes the switch to retrieve the UN data in CSV format and adds an option to download data from any of the functions which retrieve data from the UN portal. It's easy to use these retrieved data in the get_pop_objs function in a way that results in no pings of the UN portal (see the new test_demographics.py::test_data_download, which provides an example of doing this in the context of the test.

I'd noticed some odd issues in the GH Actions tests (e.g., a failure in TPI.py related to dask distributed processes, which is not affected by this PR), but the changes to the code in demographics.py are ready for review and all of the test_demographics.py test pass. I'll rerun or make necessary adjustments to ensure all tests pass.

[jdebacker]

3.9 test failure is the UN API throwing an HTTP code 403 -- a denial of access. We will see if this is specific to calls coming from 3.9 (at least one of the 3.10 tests has passed without such an error.

[rickecon]

@jdebacker. I ran all the OG-Core tests locally yesterday and last night (11 hrs, 33 min) and I only got three errors. And the three errors were assert np.allclose() or assert np.all(np.isclose()). I think we should:

• make all tests that use the download function in demographics.py be local only (@pytest.mark.local) and
• adjust the np.isclose tolerance parameter in test_txfunc.py so the tests pass on more platforms.

(ogcore-dev) richardevans@Richards-MacBook-Pro-2 OG-Core % pytest
============================= test session starts ==============================
platform darwin -- Python 3.11.8, pytest-8.0.0, pluggy-1.4.0
rootdir: /Users/richardevans/Docs/Economics/OSE/OG-Core
configfile: pytest.ini
testpaths: ./tests
plugins: xdist-3.5.0, anyio-4.2.0
collected 516 items                                                            

tests/test_SS.py .............................                           [  5%]
tests/test_TPI.py ...........................                            [ 10%]
tests/test_aggregates.py .....................................           [ 18%]
tests/test_basic.py ....                                                 [ 18%]
tests/test_demographics.py .................                             [ 22%]
tests/test_elliptical_u_est.py .......                                   [ 23%]
tests/test_execute.py .                                                  [ 23%]
tests/test_firm.py ..................................................... [ 33%]
................                                                         [ 37%]
tests/test_fiscal.py ...................                                 [ 40%]
tests/test_household.py ..............................................   [ 49%]
tests/test_output_plots.py ............................................. [ 58%]
                                                                         [ 58%]
tests/test_output_tables.py .............                                [ 60%]
tests/test_parameter_plots.py ...................................        [ 67%]
tests/test_parameter_tables.py .......                                   [ 68%]
tests/test_parameters.py ..............                                  [ 71%]
tests/test_run_example.py ..                                             [ 72%]
tests/test_run_ogcore.py .                                               [ 72%]
tests/test_tax.py ......................................                 [ 79%]
tests/test_txfunc.py .....F.......F..........F.                          [ 84%]
tests/test_user_inputs.py .........                                      [ 86%]
tests/test_utils.py .................................................... [ 96%]
..................                                                       [100%]

=================================== FAILURES ===================================
_____________________________ test_txfunc_est[DEP] _____________________________

rate_type = 'etr', tax_func_type = 'DEP', numparams = 12
expected_tuple = (array([2.27262567e-23, 6.52115408e-05, 2.58988624e-13, 5.79701075e-09,
       3.39354147e-01, 7.60132613e-01, 9.14365...1, 9.02760087e-06,
       9.02760087e-06, 3.39345119e-03, 7.60123585e-03, 9.02760087e-06]), 237677.14110076256, 152900)
tmpdir = local('/private/var/folders/d4/trj3dssd6s3g8kxvjmczz11w0000gn/T/pytest-of-richardevans/pytest-0/test_txfunc_est_DEP_0')

    @pytest.mark.local  # only marking as local because platform
    # affects results from scipy.opt that is called in this test - so it'll
    # pass if run on Mac with MKL, but not necessarily on other platforms
    @pytest.mark.parametrize(
        "rate_type,tax_func_type,numparams,expected_tuple",
        [
            ("etr", "DEP", 12, expected_tuple_DEP),
            ("etr", "DEP_totalinc", 6, expected_tuple_DEP_totalinc),
            ("etr", "GS", 3, expected_tuple_GS),
        ],
        ids=["DEP", "DEP_totalinc", "GS"],
    )
    def test_txfunc_est(
        rate_type, tax_func_type, numparams, expected_tuple, tmpdir
    ):
        """
        Test txfunc.txfunc_est() function.  The test is that given
        inputs from previous run, the outputs are unchanged.
        """
        micro_data = utils.safe_read_pickle(
            os.path.join(CUR_PATH, "test_io_data", "micro_data_dict_for_tests.pkl")
        )
        s = 80
        t = 2030
        df = txfunc.tax_data_sample(micro_data[str(t)])
        output_dir = tmpdir
        # Put old df variables into new df var names
        df.rename(
            columns={
                "MTR labor income": "mtr_labinc",
                "MTR capital income": "mtr_capinc",
                "Total labor income": "total_labinc",
                "Total capital income": "total_capinc",
                "ETR": "etr",
                "expanded_income": "market_income",
                "Weights": "weight",
            },
            inplace=True,
        )
        test_tuple = txfunc.txfunc_est(
            df, s, t, rate_type, tax_func_type, numparams, output_dir, True
        )
    
        for i, v in enumerate(expected_tuple):
>           assert np.allclose(test_tuple[i], v)
E           assert False
E            +  where False = <function allclose at 0x11225f3b0>(array([2.27262567e-23, 6.52120583e-05, 2.58990603e-13, 5.79381098e-09,\n       3.37733247e-01, 8.00000000e-01, 9.14366590e-01, 9.02760087e-06,\n       9.02760087e-06, 3.37724219e-03, 7.99990972e-03, 9.02760087e-06]), array([2.27262567e-23, 6.52115408e-05, 2.58988624e-13, 5.79701075e-09,\n       3.39354147e-01, 7.60132613e-01, 9.14365331e-01, 9.02760087e-06,\n       9.02760087e-06, 3.39345119e-03, 7.60123585e-03, 9.02760087e-06]))
E            +    where <function allclose at 0x11225f3b0> = np.allclose

tests/test_txfunc.py:253: AssertionError
...
______________________________ test_tax_func_loop ______________________________

    @pytest.mark.local
    # mark as local run since results work on Mac, but differ on other
    # platforms
    def test_tax_func_loop():
        """
        Test txfunc.tax_func_loop() function. The test is that given inputs from
        previous run, the outputs are unchanged.
        """
        input_tuple = decompress_pickle(
            os.path.join(
                CUR_PATH, "test_io_data", "tax_func_loop_inputs_large.pbz2"
            )
        )
        (
            t,
            micro_data,
            beg_yr,
            s_min,
            s_max,
            age_specific,
            analytical_mtrs,
            desc_data,
            graph_data,
            graph_est,
            output_dir,
            numparams,
            tpers,
        ) = input_tuple
        tax_func_type = "DEP"
        # Rename and create vars to suit new micro_data var names
        micro_data["total_labinc"] = (
            micro_data["Wage income"] + micro_data["SE income"]
        )
        micro_data["etr"] = (
            micro_data["Total tax liability"] / micro_data["Adjusted total income"]
        )
        micro_data["total_capinc"] = (
            micro_data["Adjusted total income"] - micro_data["total_labinc"]
        )
        # use weighted avg for MTR labor - abs value because
        # SE income may be negative
        micro_data["mtr_labinc"] = micro_data["MTR wage income"] * (
            micro_data["Wage income"]
            / (micro_data["Wage income"].abs() + micro_data["SE income"].abs())
        ) + micro_data["MTR SE income"] * (
            micro_data["SE income"].abs()
            / (micro_data["Wage income"].abs() + micro_data["SE income"].abs())
        )
        micro_data.rename(
            columns={
                "Adjusted total income": "market_income",
                "MTR capital income": "mtr_capinc",
                "Total tax liability": "total_tax_liab",
                "Year": "year",
                "Age": "age",
                "expanded_income": "market_income",
                "Weights": "weight",
            },
            inplace=True,
        )
        micro_data["payroll_tax_liab"] = 0
        test_tuple = txfunc.tax_func_loop(
            t,
            micro_data,
            beg_yr,
            s_min,
            s_max,
            age_specific,
            tax_func_type,
            analytical_mtrs,
            desc_data,
            graph_data,
            graph_est,
            output_dir,
            numparams,
        )
    
        expected_tuple = utils.safe_read_pickle(
            os.path.join(CUR_PATH, "test_io_data", "tax_func_loop_outputs.pkl")
        )
    
        for i, v in enumerate(expected_tuple):
            if isinstance(test_tuple[i], list):
                test_tuple_obj = np.array(test_tuple[i])
                exp_tuple_obj = np.array(expected_tuple[i])
                print(
                    "For element",
                    i,
                    ", diff =",
                    np.absolute(test_tuple_obj - exp_tuple_obj).max(),
                )
            else:
                print(
                    "For element",
                    i,
                    ", diff =",
                    np.absolute(test_tuple[i] - v).max(),
                )
>           assert np.allclose(test_tuple[i], v, atol=1e-06)
E           assert False
E            +  where False = <function allclose at 0x11225f3b0>([array([2.55264562e-22, 3.51268116e-05, 3.98196664e-09, 2.22091644e-04,\n       2.94518504e-01, 1.10154861e-03, 1.00000...5296e-01, 1.00000000e+00, 1.01110118e-03,\n       1.01110118e-03, 2.86635079e-03, 1.64414195e-03, 1.01110118e-03]), ...], [array([2.55264562e-22, 3.51268127e-05, 3.88349581e-09, 1.86048713e-04,\n       2.94518501e-01, 1.10154861e-03, 1.00000...0733e-01, 1.00000000e+00, 1.01110118e-03,\n       1.01110118e-03, 2.86635057e-03, 1.67539632e-03, 1.01110118e-03]), ...], atol=1e-06)
E            +    where <function allclose at 0x11225f3b0> = np.allclose

tests/test_txfunc.py:434: AssertionError
...
____________________________ test_tax_func_estimate ____________________________

tmpdir = local('/private/var/folders/d4/trj3dssd6s3g8kxvjmczz11w0000gn/T/pytest-of-richardevans/pytest-0/test_tax_func_estimate0')
dask_client = <Client: 'tcp://127.0.0.1:58830' processes=2 threads=4, memory=64.00 GiB>

    @pytest.mark.local
    def test_tax_func_estimate(tmpdir, dask_client):
        """
        Test txfunc.tax_func_loop() function.  The test is that given
        inputs from previous run, the outputs are unchanged.
        """
        input_tuple = utils.safe_read_pickle(
            os.path.join(CUR_PATH, "test_io_data", "tax_func_estimate_inputs.pkl")
        )
        micro_data = utils.safe_read_pickle(
            os.path.join(CUR_PATH, "test_io_data", "micro_data_dict_for_tests.pkl")
        )
        (
            BW,
            S,
            starting_age,
            ending_age,
            beg_yr,
            baseline,
            analytical_mtrs,
            age_specific,
            reform,
            data,
            client,
            num_workers,
        ) = input_tuple
        tax_func_type = "DEP"
        age_specific = False
        BW = 1
        test_path = os.path.join(tmpdir, "test_out.pkl")
        test_dict = txfunc.tax_func_estimate(
            micro_data,
            BW,
            S,
            starting_age,
            ending_age,
            start_year=2030,
            baseline=baseline,
            analytical_mtrs=analytical_mtrs,
            tax_func_type=tax_func_type,
            age_specific=age_specific,
            reform=reform,
            data=data,
            client=dask_client,
            num_workers=NUM_WORKERS,
            tax_func_path=test_path,
        )
        expected_dict = utils.safe_read_pickle(
            os.path.join(CUR_PATH, "test_io_data", "tax_func_estimate_outputs.pkl")
        )
        del expected_dict["tfunc_time"]
    
        for k, v in expected_dict.items():
            if isinstance(v, str):  # for testing tax_func_type object
                assert test_dict[k] == v
            elif isinstance(expected_dict[k], list):
                test_dict_obj = np.array(test_dict[k])
                exp_dict_obj = np.array(expected_dict[k])
                print(
                    "For element",
                    k,
                    ", diff =",
                    np.absolute(test_dict_obj - exp_dict_obj).max(),
                )
            else:  # for testing all other objects
                print(
                    "Max diff for ", k, " = ", np.absolute(test_dict[k] - v).max()
                )
>               assert np.all(np.isclose(test_dict[k], v))
E               assert False
E                +  where False = <function all at 0x112257370>(array([[ True,  True,  True,  True,  True,  True,  True,  True,  True,\n         True,  True,  True,  True,  True,  Tru...rue,  True,  True,  True,  True,  True,  True,  True,\n         True,  True,  True,  True,  True,  True,  True,  True]]))
E                +    where <function all at 0x112257370> = np.all
E                +    and   array([[ True,  True,  True,  True,  True,  True,  True,  True,  True,\n         True,  True,  True,  True,  True,  Tru...rue,  True,  True,  True,  True,  True,  True,  True,\n         True,  True,  True,  True,  True,  True,  True,  True]]) = <function isclose at 0x11225f4f0>(array([[     0.        ,      0.        ,      0.        ,\n             0.        ,      0.        ,      0.        ,\n...     0.        ,\n             0.        ,      0.        ,      0.        ,\n             0.        ,      0.        ]]), array([[     0.        ,      0.        ,      0.        ,\n             0.        ,      0.        ,      0.        ,\n...     0.        ,\n             0.        ,      0.        ,      0.        ,\n             0.        ,      0.        ]]))
E                +      where <function isclose at 0x11225f4f0> = np.isclose

tests/test_txfunc.py:722: AssertionError
...
=========================== short test summary info ============================
FAILED tests/test_txfunc.py::test_txfunc_est[DEP] - assert False
FAILED tests/test_txfunc.py::test_tax_func_loop - assert False
FAILED tests/test_txfunc.py::test_tax_func_estimate - assert False
========= 3 failed, 513 passed, 16816 warnings in 41615.52s (11:33:35) =========

[rickecon]

@jdebacker. The three test_txfunc.py tests that are failing on my full pytest run on my local machine are all marked to only run on local tests. So we don't need to worry about those issues for this PR. However, I think we do need to get rid of the data download in demographics.py in the CI tests that run on GitHub. I think those http errors are what are blowing up the dask workers. We should use downloaded data for the tests that run on GitHub. So we should not have any GitHub CI tests that call the get_un_data() function in demographics.py.

[rickecon]

@jdebacker. I think we should merge this PR with the errors. Then create an issue documenting the errors. Then I would propose opening a new PR that makes all tests that access the get_un_data() function run only locally and makes all the tests that run on the GH CI use saved UN population data.

[jdebacker]

@rickecon Thanks for your review!

Latest commits make your proposed changes (I think) -- I marked all tests that I think talk to the data portal as "local" and use cached data for that other tests.

I hope to have all CI tests passing before merging this PR.

[jdebacker]

Tests of demographics.py ok now. But still getting weird failures in TPI:

FAILED tests/test_TPI.py::test_run_TPI[Baseline] - concurrent.futures._base.CancelledError: root-3e362076-3158-4fe9-a38a-2cbd498c2091

Any ideas @rickecon ?

[rickecon]

@jdebacker. The error in all three cases is distributed.worker - ERROR - Could not find data: ['Specifications-5eb283f0acecd3e9c42a9de8dd1bb382'] where the long hash number at the end varies across tests. This suggests to me that the demographic data might not be loaded or located properly for the test.

[rickecon]

@jdebacker. The two TPI tests that are failing, I think are failing because they require the new demographic data files to already be in the repository master file folder. With this hypothesis, I am going to open and merge PR #907 that puts these files in the /tests/test_io_data/ directory. Then all the tests should pass.

[rickecon]

@jdebacker. By adding the demographics data files to the ./tests/test_io_data/ folder in PR #907, I was able to get the Linux Python 3.10 tests to pass. However, the Linux Python 3.11 tests had one error in test_TPI.py and one error in test_demographics.py. I currently have the Mac Python 3.10 tests running. We'll see how those go.

In the meantime, you should update the version in setup.py and ./ogcore/__init__.py to 0.11.3. And also make an update to CHANGELOG.md both at the top of the file as well as the markdown links at the bottom of the file.

[jdebacker]

That's all odd - given I had added those data in this PR, I think the tests should be pulling the cached demographic data from this branch just fine for the test. I did see a failure with an HTTP 429 error as I forgot to mark a test that does ping the UN. Just fixed that.

The error in test_TPI.py seem like there might be something technical with how the local cluster is working for some of the GH Action instances:

    async def _gather(self, futures, errors="raise", direct=None, local_worker=None):
        unpacked, future_set = unpack_remotedata(futures, byte_keys=True)
        mismatched_futures = [f for f in future_set if f.client is not self]
        if mismatched_futures:
            raise ValueError(
                "Cannot gather Futures created by another client. "
                f"These are the {len(mismatched_futures)} (out of {len(futures)}) "
                f"mismatched Futures and their client IDs (this client is {self.id}): "
                f"{ {f: f.client.id for f in mismatched_futures} }"  # noqa: E201, E202
            )
        keys = [future.key for future in future_set]
        bad_data = dict()
        data = {}

[rickecon]

@jdebacker. I am still re-running tests one-by-one, although both Mac OS Python 3.10 and Python 3.11 have now failed. I don't know what the issue is, and I can't find any related GitHub issues, Dask issues, or StackOverflow threads that are related to this (e.g., "GitHub Action Dask concurrent.futures._base.CancelledError").

This problem seems to have arisen with this most recent pull request. And I don't think it is related to the content of this PR, although iI guess it could still be some rogue calls to the UN API. I recommend we try the following changes.

1. Reduce the number of workers in our parallel processing in all of our test .py files. For example, change line 16 in test_SS.py to be the following. And make that change in all the .py files in the ./tests/ directory

NUM_WORKERS = min(multiprocessing.cpu_count(), 4)

2. Get rid of the Python 3.9 tests in our CI. I don't think that this is the problem, but I don't think we need to test Python 3.9 anymore. And the shorter compute here might counter the potentially longer compute from the change above. And it would just be nice if all our GH Actions took a little less long. And I am a little sheepish about how much compute we are using.

[SeaCelo]

@rickecon @jdebacker We are in touch with the API developers to see what is causing the errors. They are asking for some code that replicates it. Can you share something that they can run in python to test?

cc: @sezdi0810

[rickecon]

@jdebacker. We know this passes tests locally. We can make an issue about these tests not passing. Merging.