Allow image comparison outside tests module #5842

Merged
merged 2 commits into from Jan 13, 2016

Conversation

Projects
None yet
4 participants
Contributor

maxalbert commented Jan 13, 2016

This is a replacement for PR #5518 (which should be closed if this is accepted). For now this PR simply removes the assert statement so that the @image_comparison decorator can be used in situations where the test does not live in a parent directory called tests (this does not apply to matplotlib but will be useful for third-party users).

In the medium term it would be good to re-think and refactor the code in _image_directories because isn't very easy to read at a glance if you don't know what's going on already, and the logic seems somewhat fragile (see #3314 for example).

maxalbert added some commits Jan 13, 2016

@maxalbert maxalbert Expand docstring to explain return value. 27cbd34
@maxalbert maxalbert Remove assertion so that @image_comparison can be used outside a 'tes…
…ts' module, but issue warning because the code logic seems a bit fragile and may fail.
90b8f11

mdboom added the needs_review label Jan 13, 2016

Contributor

maxalbert commented Jan 13, 2016

There is one failure in the Travis build but it seems to be the PEP8 issue that was fixed by #5844. Not sure what's making the AppVeyor builds fail to be honest.

tacaswell closed this Jan 13, 2016

tacaswell reopened this Jan 13, 2016

@tacaswell tacaswell added needs_review and removed needs_review labels Jan 13, 2016

Owner

tacaswell commented Jan 13, 2016

power cycled the PR to re-kick CI against current master.

Owner

tacaswell commented Jan 13, 2016

👍 from me once travis passes. This should be non-controversial and I think be backported back to v1.5.x

Member

WeatherGod commented Jan 13, 2016

appveyor failed on a couple of the job runs with a strange error. I can't restart them.

Owner

tacaswell commented Jan 13, 2016

Do not worry about appveyor too, it also lies that it passes ;)

On Wed, Jan 13, 2016 at 1:24 PM Benjamin Root notifications@github.com
wrote:

appveyor failed on a couple of the job runs with a strange error. I can't
restart them.


Reply to this email directly or view it on GitHub
#5842 (comment)
.

Member

WeatherGod commented Jan 13, 2016

Ok. Merging and backporting to v1.5.x. Do I also need to manually backport it to the v2.x branch, or will that happen on its own?

@WeatherGod WeatherGod added a commit that referenced this pull request Jan 13, 2016

@WeatherGod WeatherGod Merge pull request #5842 from maxalbert/allow_image_comparison_outsid…
…e_tests_module

Allow image comparison outside tests module
c446a64

@WeatherGod WeatherGod merged commit c446a64 into matplotlib:master Jan 13, 2016

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

mdboom removed the needs_review label Jan 13, 2016

@WeatherGod WeatherGod added a commit that referenced this pull request Jan 13, 2016

@WeatherGod WeatherGod Merge pull request #5842 from maxalbert/allow_image_comparison_outsid…
…e_tests_module

Allow image comparison outside tests module
5240759
Member

WeatherGod commented Jan 13, 2016

backported to v1.5.x as 5240759

Owner

tacaswell commented Jan 13, 2016

every so someone does a 1.5.x -> 2.x -> master cascade

@tacaswell tacaswell added a commit to tacaswell/matplotlib that referenced this pull request May 22, 2016

@WeatherGod @tacaswell WeatherGod + tacaswell Merge pull request #5842 from maxalbert/allow_image_comparison_outsid…
…e_tests_module

Allow image comparison outside tests module
059bca3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment