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

Introductory py.test failing #1086

Closed
salimfurth opened this issue Dec 5, 2016 · 21 comments
Closed

Introductory py.test failing #1086

salimfurth opened this issue Dec 5, 2016 · 21 comments
Assignees

Comments

@salimfurth
Copy link
Contributor

salimfurth commented Dec 5, 2016

The test of tax calculator, as suggested on the Contributor Guide, is giving 4 failures in the up-to-date tax calculator. Both @kdd0211 and I encountered this problem today, and we verified it on 3 separate systems. On all 3 systems, the tests give 4 failures.

Here's the output from @kdd0211's interface:

MINGW64 ~/Desktop/tax-calculator/taxcalc (master)
$ py.test -m "not requires_pufcsv"

============================= test session starts =============================
platform win32 -- Python 3.5.2, pytest-2.8.5, py-1.4.31, pluggy-0.3.1
rootdir: C:\Users\kdd0211\Desktop\tax-calculator, inifile: setup.cfg
collected 298 items

tests\test_behavior.py .......
tests\test_calculate.py ..................
tests\test_consumption.py .......
tests\test_decorators.py ...............F
tests\test_dropq.py ...................F.F..........
tests\test_functions.py ....
tests\test_growth.py ......
tests\test_incometaxio.py ..............
tests\test_macro_elasticity.py .
tests\test_parameters.py .
tests\test_policy.py ..........................
tests\test_records.py ..............
tests\test_reforms.py .
tests\test_simpletaxio.py ...................
tests\test_utils.py ...................................F......
tests\filings\forms\test_tax_form.py .............
tests\filings\forms\test_tax_forms.py .......................................... ................................

================================== FAILURES ===================================
_____________________________ test_force_no_numba _____________________________

def test_force_no_numba():
    """
    Force execution of code for non-existence of Numba
    """
    global Magic_calc6

    # Mock the numba module
  from mock import Mock

E ImportError: No module named 'mock'

tests\test_decorators.py:296: ImportError
___________________________ test_create_json_table ____________________________

def test_create_json_table():
    df = DataFrame(data=[[1., 2, 3], [4, 5, 6], [7, 8, 9]],
                   columns=['a', 'b', 'c'])
  ans = create_json_table(df)

tests\test_dropq.py:342:


dropq\dropq_utils.py:67: in create_json_table
row_out.append(format_print(df.loc[idx, col], _type, num_decimals))


x = 2.0, _type = dtype('int64'), num_decimals = 2

def format_print(x, _type, num_decimals):
    float_types = [float, np.dtype('f8')]
    frmat_str = "0:.{num}f".format(num=num_decimals)
    frmat_str = "{" + frmat_str + "}"
    try:
        if _type in float_types or _type is None:
            return frmat_str.format(x)
        elif _type == int:
            return str(int(x))
        elif _type == str:
            return str(x)
        else:
          raise NotImplementedError()

E NotImplementedError

dropq\dropq_utils.py:21: NotImplementedError
______________________ test_format_print_not_implemented ______________________

def test_format_print_not_implemented():
    x = np.array([1], dtype='i4')
    with pytest.raises(NotImplementedError):
      format_print(x[0], x.dtype, 2)

E Failed: DID NOT RAISE

tests\test_dropq.py:361: Failed
_____________________________ test_xtr_graph_plot _____________________________

records_2009 = <taxcalc.records.Records object at 0x0000001956E60D30>

def test_xtr_graph_plot(records_2009):
    calc = Calculator(policy=Policy(),
                      records=records_2009,
                      behavior=Behavior())
    gdata = mtr_graph_data(calc, calc, mtr_measure='ptax',
                           income_measure='agi',
                           dollar_weighting=False)
  gplot = xtr_graph_plot(gdata)

tests\test_utils.py:597:


utils.py:1007: in wrapped_f
return func(*args, **kwargs)


data = {'lines': base reform
0 0 0
1 0 0
2 0 0
3 0 0
4 ... Marginal Tax Rate for e00200p by Income Percentile for 2013', 'xlabel': ' AGI Percentile', 'ylabel': 'Payroll-Tax MTR'}
width = 850, height = 500, xlabel = '', ylabel = ''
title = 'Mean Marginal Tax Rate for e00200p by Income Percentile for 2013'
legendloc = 'bottom_right'

@requires_bokeh
def xtr_graph_plot(data,
                   width=850,
                   height=500,
                   xlabel='',
                   ylabel='',
                   title='',
                   legendloc='bottom_right'):
    """
    Plot marginal/average tax rate graph using data returned from either the
    mtr_graph_data function or the atr_graph_data function.

    Parameters
    ----------
    data : dictionary object returned from ?tr_graph_data() utility function

    width : integer
        width of plot expressed in pixels

    height : integer
        height of plot expressed in pixels

    xlabel : string
        x-axis label; if '', then use label generated by ?tr_graph_data

    ylabel : string
        y-axis label; if '', then use label generated by ?tr_graph_data

    title : string
        graph title; if '', then use title generated by ?tr_graph_data

    legendloc : string
        options: 'top_right', 'top_left', 'bottom_left', 'bottom_right'
        specifies location of the legend in the plot

    Returns
    -------
    bokeh.plotting figure object containing a raster graphics plot

    Notes
    -----
    USAGE EXAMPLE:
      gdata = mtr_graph_data(calc1, calc2)
      gplot = xtr_graph_plot(gdata)
    THEN  # when working interactively in a Python notebook
      bp.show(gplot)
    OR    # when executing script using Python command-line interpreter
      bp.output_file('anyname.html')  # file to write to when invoking bp.sh                                                                                                                                  ow
      bp.show(gplot, title='MTR by Income Percentile')
    WILL VISUALIZE GRAPH IN BROWSER

    To convert the visualized graph into a PNG-formatted file, click on
    the "Save" icon on the Toolbar (located in the top-right corner of
    the visualized graph) and a PNG-formatted file will written to your
    Download directory.

    The ONLY output option the bokeh.plotting figure has is HTML format,
    which (as described above) can be converted into a PNG-formatted
    raster graphics file.  There is no option to make the bokeh.plotting
    figure generate a vector graphics file such as an EPS file.
    """
    # pylint: disable=too-many-arguments
    if title == '':
        title = data['title']
    fig = bp.figure(plot_width=width, plot_height=height, title=title)
  fig.title.text_font_size = '12pt'

E AttributeError: 'str' object has no attribute 'text_font_size'

utils.py:1081: AttributeError
---------------------------- Captured stdout call -----------------------------
You loaded data for 2009.
Your data have been extrapolated to 2013.
============== 3 tests deselected by "-m 'not requires_pufcsv'" ===============
============ 4 failed, 291 passed, 3 deselected in 110.11 seconds =============

@martinholmer
Copy link
Collaborator

@salimfurth said:

The test of tax calculator, as suggested on the Contributor Guide, is giving 4 failures in the up-to-date tax calculator. Both @kdd0211 and I encountered this problem today, and we verified it on 3 separate systems. On all 3 systems, the tests give 4 failures.

Here's the output from @kdd0211's interface: ...

Not sure about what is going on, but one possibility is that you don't have all the Python packages that Tax-Calculator expects (or you might have the packages but they are not up-to-date).

Two of your errors seem to be related to the mock and bokeh packages. All the tests pass on my computer where I get the following:

iMac2:tax-calculator mrh$ conda list | grep bokeh
bokeh                     0.12.3                   py27_0  
iMac2:tax-calculator mrh$ conda list | grep mock
mock                      1.0.1                    py27_0 

Do you have mock installed?
Do you have bokeh version 0.12.3?
If not, then you need to install them or upgrade to the expected version.
I think the Contributor Guide tells you how to do that; if not, ask here for more info.

@talumbau
Copy link
Member

talumbau commented Dec 6, 2016

At first, I thought that the culprit would be that mock and bokeh are missing in the environment.yml file, but they are both listed in the file in master. As @martinholmer suggested though, it would be good to know if those packages are in the environment.

@talumbau
Copy link
Member

talumbau commented Dec 6, 2016

Did you have a pre-existing environment named taxcalc-dev, then pull master, and then run the tests? When I look at the blame of the environment file, it looks like mock and bokeh are the most recently added. If that is the case, you would need to execute:

conda env update

at the top of the repository. This will find your pre-existing taxcalc-dev environment, look at what is new in the environment.yml file, and then update/get new packages as needed.

@salimfurth
Copy link
Contributor Author

Thanks @martinholmer and @talumbau, installing bokeh and mock followed by conda env update fixed two of the 4 failures. The ones that remain are in test_create_json_table() and then one that seems to be a result of that failure, test_format_print_not_implemented().

@talumbau
Copy link
Member

talumbau commented Dec 6, 2016

this is good to know. Looking more closely at your error message, I'm wondering if you could specify the three systems where you are seeing the failures. The main information I need is operating system and Python version. Thanks in advance

@salimfurth
Copy link
Contributor Author

Thanks @talumbau. All three are using Python version 2.7.12 and a Windows OS.

  • @kdd0211's laptop has Windows 8
  • My brand new laptop has Windows 10 Home
  • The CDA workstation has Windows 7 Enterprise.

It's worth noting that even the 12/5 install of Anaconda on my home computer, following the Contributors Guide, didn't give me up-to-date mock or bokeh. I suggest adding those command lines in the appropriate part of the the Contributors Guide.

@martinholmer
Copy link
Collaborator

martinholmer commented Dec 6, 2016

@salimfurth said:

It's worth noting that even the 12/5 install of Anaconda on my home computer, following the Contributors Guide, didn't give me up-to-date mock or bokeh. I suggest adding those command lines in the appropriate part of the the Contributors Guide.

Thanks for reporting your installation experience. But actually, this is not a documentation bug, but an apparent bug in the installation procedure. We should investigate that installation bug and fix it.

@talumbau, Is the installation procedure tested under Windows?

@MattHJensen

@martinholmer
Copy link
Collaborator

@salimfurth, Have you been able to resolve the unit test failures on your Windows computers?
@talumbau, Have you had a chance to look into this yet? (I know you are handling quite a few things right now.)

@talumbau
Copy link
Member

talumbau commented Dec 7, 2016

It looks like the two errors that are now resolved are due to pre-existing conda environments that needed to be updated. It might make sense to update the documentation to tell people to execute conda env update if they already have the conda environment taxcalc-dev on their machine.

For the two other errors, these could be legitimate Windows errors. I have a VM to try things out. We need to add AppVeyor in addition to Travis CI testing. AppVeyor does exactly what Travis CI does but Travis CI is only for Linux platforms.

@martinholmer
Copy link
Collaborator

@talumbau said:

It looks like the two errors that are now resolved are due to pre-existing conda environments that needed to be updated. It might make sense to update the documentation to tell people to execute conda env update if they already have the conda environment taxcalc-dev on their machine.

OK. That documentation update has been done in pull request #1090.

@talumbau said:

For the two other errors, these could be legitimate Windows errors. I have a VM to try things out. We need to add AppVeyor in addition to Travis CI testing. AppVeyor does exactly what Travis CI does but Travis CI is only for Linux platforms.

I don't understand your comment. The errors they are getting are on their local Windows computers, not when they are trying to submit a pull request to GitHub, which is where Travis-CI comes into play.

@salimfurth

@talumbau
Copy link
Member

talumbau commented Dec 7, 2016

It's correct that Travis CI is for testing pull requests, but the only way code is allowed to get into master is through pull requests showing that all the tests pass. So, if these errors are legitimate, they were introduced at some point in the past by code that passed all tests on Linux and Mac (so it was allowed in to master), but testing on Windows through AppVeyor would have shown the failures. I should be able to confirm these errors shortly.

@martinholmer
Copy link
Collaborator

martinholmer commented Dec 8, 2016

@talumbau said in the #1086 conversation:

It's correct that Travis CI is for testing pull requests, but the only way code is allowed to get into master is through pull requests showing that all the tests pass. So, if these errors are legitimate, they were introduced at some point in the past by code that passed all tests on Linux and Mac (so it was allowed in to master), but testing on Windows through AppVeyor would have shown the failures. I should be able to confirm these errors shortly.

So, am I right in concluding that the @zrisher analysis of issue #1086 and his fixes in #1092 and #1094 mean that your notion that we need something other than Travis-CI on GitHub is unnecessary? All the problems reported in #1086 seem to be related to different package versions.

@talumbau
Copy link
Member

talumbau commented Dec 8, 2016

I don't know that I would draw that conclusion ('Windows testing is unnecessary') even if the issue is related to package versions. Right now, it seems the best leading theory is that executing the contributor guide instructions results in the installation of pandas 0.19, which causes two tests to fail because it reveals a previously ignored keyword argument. But in #1091 @zrisher says the failures also occur in pandas 0.18.1, which I think might be a typo, given that the issue name is "Tests fail on pandas 0.19.1 ". I'm waiting on his response so that I understand more fully what is happening.

@zrisher
Copy link
Contributor

zrisher commented Dec 9, 2016

@talumbau @martinholmer @salimfurth The remaining test failures mentioned in this issue are the same that I'm working on in #1091 for #1095. I hope to have a fix for those soon.

@talumbau
Copy link
Member

I can confirm I'm getting the remaining two errors on Windows 7. So these are real errors introduced when I put dropq in, but didn't catch because I didn't test on Windows.

@martinholmer
Copy link
Collaborator

@talumbau said with respect issue #1086:

I can confirm I'm getting the remaining two errors on Windows 7. So these are real errors introduced when I put dropq in[to the tax-calculator repo], but didn't catch because I didn't test on Windows.

Thanks for the report, @talumbau. So, what is the next step? We do need to allow Windows users to participate in the Tax-Calculator project.

@talumbau
Copy link
Member

Windows users can participate in the Tax-Calculator project right now. The only known issue is that some code that formats output for some dropq functions may work differently from expected, or it may be that it was written with certain assumptions that don't hold on Windows. But we should fix these functions for Windows. The path forward would be:

  • someone posts a fix for the two failing tests on Windows
  • someone adds code that sets up this repo to be tested with AppVeyor

Those two actions don't have to happen at the same time, but both should happen as soon as possible.

@feenberg
Copy link
Contributor

feenberg commented Dec 16, 2016 via email

@talumbau
Copy link
Member

that's correct. So the issue should not inhibit anyone from typical modeling use cases on a Windows machine. Still, these failure are good to know and we should fix them.

@talumbau talumbau self-assigned this Dec 16, 2016
@talumbau
Copy link
Member

I have this problem fixed in #1111, which also adds in AppVeyor to make sure we are testing with Windows on every pull request. Thanks for finding and reporting this bug @salimfurth

@talumbau
Copy link
Member

closed via #1111

@talumbau talumbau removed the ready label Dec 29, 2016
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

No branches or pull requests

5 participants