Skip to content

Updated bundled py.test to 2.8.3 - #4349

Merged
eteq merged 7 commits into
astropy:masterfrom
astrofrog:update-bundled-pytest
Feb 12, 2016
Merged

eteq merged 7 commits into
astropy:masterfrom
astrofrog:update-bundled-pytest

Conversation

@astrofrog

Copy link
Copy Markdown
Member

This updates the bundled py.test to 2.8.3 and fixes the issue reported by @bsipocz: #3499

The easiest way to reproduce that original issue is:

python -c 'from astropy import test; test("modeling"); test("modeling")'

However, there are still some other (unrelated) errors in the second run:

...
Platform: Darwin-12.5.0-x86_64-i386-64bit

Executable: /Users/tom/miniconda3/envs/production/bin/python

Full Python Version: 
3.4.3 |Continuum Analytics, Inc.| (default, Oct 20 2015, 14:27:51) 
[GCC 4.2.1 (Apple Inc. build 5577)]

encodings: sys: utf-8, locale: UTF-8, filesystem: utf-8
byteorder: little
float info: dig: 15, mant_dig: 15

Numpy: 1.10.1
Scipy: 0.16.0
Matplotlib: 1.5.0
h5py: 2.5.0
...
========================================================================================== FAILURES ==========================================================================================
______________________________________________________________________________ test_inherit_constraints[model0] ______________________________________________________________________________

model = <CompoundModel11(amplitude_0=1.0, mean_0=0.0, stddev_0=1.0, amplitude_1=1.0, mean_1=0.0, stddev_1=1.0)>

    @pytest.mark.parametrize('model',
            [Gaussian1D(bounds={'stddev': (0, 0.3)}, fixed={'mean': True}) +
                Gaussian1D(fixed={'mean': True}),
             (_ConstraintsTestA + _ConstraintsTestB)()])
    def test_inherit_constraints(model):
        """
        Various tests for copying of constraint values between compound models and
        their members.

        There are two versions of this test: One where a compound model is created
        from two model instances, and another where a compound model is created
        from two model classes that have default constraints set on some of their
        parameters.

        Regression test for https://github.com/astropy/astropy/issues/3481
        """

        # Lots of assertions in this test as there are multiple interfaces to
        # parameter constraints

        assert 'stddev_0' in model.bounds
>       assert model.bounds['stddev_0'] == (0, 0.3)
E       assert (0.1, 0.5) == (0, 0.3)
E         At index 0 diff: 0.1 != 0
E         Use -v to get the full diff

miniconda3/envs/production/lib/python3.4/site-packages/astropy-1.1.dev14144-py3.4-macosx-10.5-x86_64.egg/astropy/modeling/tests/test_compound.py:759: AssertionError
______________________________________________________________________________ test_inherit_constraints[model1] ______________________________________________________________________________

model = <CompoundModel12(stddev_0=0.0, mean_0=0.0, mean_1=0.0)>

    @pytest.mark.parametrize('model',
            [Gaussian1D(bounds={'stddev': (0, 0.3)}, fixed={'mean': True}) +
                Gaussian1D(fixed={'mean': True}),
             (_ConstraintsTestA + _ConstraintsTestB)()])
    def test_inherit_constraints(model):
        """
        Various tests for copying of constraint values between compound models and
        their members.

        There are two versions of this test: One where a compound model is created
        from two model instances, and another where a compound model is created
        from two model classes that have default constraints set on some of their
        parameters.

        Regression test for https://github.com/astropy/astropy/issues/3481
        """

        # Lots of assertions in this test as there are multiple interfaces to
        # parameter constraints

        assert 'stddev_0' in model.bounds
>       assert model.bounds['stddev_0'] == (0, 0.3)
E       assert (0.1, 0.5) == (0, 0.3)
E         At index 0 diff: 0.1 != 0
E         Use -v to get the full diff

miniconda3/envs/production/lib/python3.4/site-packages/astropy-1.1.dev14144-py3.4-macosx-10.5-x86_64.egg/astropy/modeling/tests/test_compound.py:759: AssertionError
===================================================================== 2 failed, 486 passed, 34 skipped in 31.85 seconds ======================================================================

@embray @nden - it looks like there must be some internal state that gets preserved between runs?

Note also that --genscript has been deprecated in py.test:

pytest-dev/pytest#884

So we may want to consider actually removing the bundled version and simply requiring py.test to be installed in order to run the tests. We could always consider this to be the last upgrade of the bundled version, and next time something breaks and requires a more recent py.test, we can then remove it?

@astrofrog

Copy link
Copy Markdown
Member Author

Also, this is not 1.1-critical so should not be backported and should not hold up the final release.

@bsipocz

bsipocz commented Nov 25, 2015

Copy link
Copy Markdown
Member

thanks @astrofrog, i'll try this out to see how much of those extra 25 fails go away :)

@embray

embray commented Nov 25, 2015

Copy link
Copy Markdown
Member

Well whatever the case this should be mentioned in the changelog for some release.

@embray

embray commented Nov 25, 2015

Copy link
Copy Markdown
Member

(Or more specifically, should be explicitly targetted for a release.)

@bsipocz

bsipocz commented Nov 25, 2015

Copy link
Copy Markdown
Member

@astrofrog - When I run the full test suite twice with this PR, I still see 29 fails and 2 errors. Full log here: https://gist.github.com/bsipocz/0f37ec40b7c929044fee

@astrofrog

Copy link
Copy Markdown
Member Author

Yeah, this is very much a work in progress - it actually creates more failures than it fixes. I don't have time to investigate that this week, so if someone would like to take this over, please feel free to (but this is not 1.1 critical and there is no actual bug in the astropy code)

@astrofrog
astrofrog force-pushed the update-bundled-pytest branch 2 times, most recently from 93324c2 to d3393a2 Compare January 1, 2016 20:18
@astrofrog astrofrog changed the title WIP: Updated bundled py.test to 2.8.3 Updated bundled py.test to 2.8.3 Jan 1, 2016
@astrofrog

Copy link
Copy Markdown
Member Author

@embray - could you review the changes to the pytest plugins since I think you are familiar with them?

@bsipocz - can you check if this fixes your original issue?

@bsipocz

bsipocz commented Jan 1, 2016

Copy link
Copy Markdown
Member

@astrofrog - Nope, it still fails even with your minimal example above (with the same failures).

@astrofrog
astrofrog force-pushed the update-bundled-pytest branch 2 times, most recently from a15992e to 2d13552 Compare January 3, 2016 22:39
@astrofrog
astrofrog force-pushed the update-bundled-pytest branch 2 times, most recently from 0268bf7 to c97e958 Compare January 7, 2016 14:20
@astrofrog

Copy link
Copy Markdown
Member Author

There are still some failures that occur when the tests are run twice, but I've opened a separate issue for those: #4444 - in the mean time, it would be good to merge this PR once it passes, since it contains some fixes that are needed if testing affiliated packages with a recent version of py.test (the plugins need to include the updates).

@astrofrog
astrofrog force-pushed the update-bundled-pytest branch from c97e958 to e3cd103 Compare January 9, 2016 11:44
@astrofrog

Copy link
Copy Markdown
Member Author

@embray @eteq - this fixes most issues with running the tests twice. There are still a couple related to warnings, but can we go ahead and merge this in the mean time, since it is already an improvement? (and fixes #3499 and #4256)

@astrofrog

Copy link
Copy Markdown
Member Author

The remaining failures are due to the fact that if you run the tests twice, the tests that ensure the deprecation warnings are raised (e.g. test_import_warning in various utils modules) don't appear the second time. However, resetwarnings is not the right approach since it resets all warning filters too. What's the best way to just make sure that the warning is raised a second time?

eteq added a commit that referenced this pull request Feb 12, 2016
@eteq
eteq merged commit f996b12 into astropy:master Feb 12, 2016
@eteq eteq added this to the v1.2.0 milestone Feb 12, 2016
@eteq

eteq commented Feb 12, 2016

Copy link
Copy Markdown
Member

Agreed that it makes sense to merge this now. I have a thought about the no-warnings-second-time problem that I'm going to investigate, but if I can't make it work I'll make an issue so that we can investigate it later.

There's some question here about backporting... My feeling is that this should only get backported if it actually fixes failures in the 1.0.x or 1.1.x - am I understanding right from the above that this fixes failures from double-invocation, but that it still runs fine on the standard python setup.py test? Or is #4256 also happening with py3.5 on the older versions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants