Consider using pytest.importorskip('dependency') in testing

There is a helper function in pytest importorskip that might help clean up testing code. This would generally replace something like @skipif('not HAS_DEPENDENCY') with a line inside the test function / method dependency = pytest.importorskip('dependency').

I'm not completely sure this is an overall win, but we should think about this as a new standard idiom.

Can this be used on a function by function basis? I thought it was just for skipping all the tests in a file but I could be wrong.

"You can use the following import helper at module level or within a test or test setup function:"

@taldcroft

Good point @taldcroft, I also assumed it skips all the files. You can assign this to me as a post1.3 task :)

In Gammapy and ctapipe we're using this utility function:
http://docs.gammapy.org/en/latest/api/gammapy.utils.testing.requires_dependency.html

The implementation is just a few lines and has been working well for us for ~ a year:

def requires_dependency(name):
    """Decorator to declare required dependencies for tests.

    Examples
    --------

    ::

        from gammapy.utils.testing import requires_dependency

        @requires_dependency('scipy')
        def test_using_scipy():
            import scipy
            ...
    """
    if name in _requires_dependency_cache:
        skip_it = _requires_dependency_cache[name]
    else:
        try:
            __import__(name)
            skip_it = False
        except ImportError:
            skip_it = True

        _requires_dependency_cache[name] = skip_it

    reason = 'Missing dependency: {}'.format(name)
    return pytest.mark.skipif(skip_it, reason=reason)

Personally, I find the declarative approach of declaring requirements via decorators on tests nicer than a

dependency = pytest.importorskip('dependency')

line at the start of the test. I could make a PR if others think it's a good solution.

Which do you prefer?

@cdeil

I like the decorator approach (maybe I'm just better used to it), as it's easier and quicker to see the scenarios when a test is skipped.
@cdeil - Do you use this also for checking the minimum version number, or missing functionality (e.g. some of the scipy functions only exists when their optional dependencies are also installed)? Nailing all of these would make it a very powerful tool.

If it did all of those it should be made into a pytest plugin, as that would be way more useful than just astro type things.

@cdeil

There is a similar discussion in #4630, with the idea to use the decorator from glue (https://github.com/glue-viz/glue/blob/master/glue/tests/helpers.py#L14), very similar to @cdeil 's one.

@bsipocz

Do you use this also for checking the minimum version number, or missing functionality ...?

No and no.
At the moment we're using it for the common simple case:
https://github.com/gammapy/gammapy/search?utf8=%E2%9C%93&q=requires_dependency&type=Code

Version number support should be easy to implement (there's already utils functions for parsing out version numbers we can re-use)

Probably checks for optional functions could be added as well, maybe even a mini-language to see if they are callable with dummy arguments. Probably not something I would add in the first version.

If it did all of those it should be made into a pytest plugin, as that would be way more useful than just astro type things.

I agree: I would suggest to start with an email to the pytest mailing list asking:

why they decided to put this as a marker in the function: http://doc.pytest.org/en/latest/skipping.html#skipping-on-a-missing-import-dependency
if they agree that having it as a decorator on the test function instead is nice / OK
if that exists as a pytest plugin already or if they think it would be a good idea to have this in a pytest plugin.

@bsipocz - Agree? You or me?

@saimn

@saimn - Thanks for pointing out the other issue and similar glue solution.

Might even be appropriate for pytest core, as a new convenience and alternative to importorskip.
Again -- we should talk to the pytest folk before taking a decision / action IMO.

@cdeil

@cdeil - you go ahead, I was just sharing a wishlist :)

I've written to the testing-in-python list about this, the thread starts here:
http://lists.idyll.org/pipermail/testing-in-python/2016-December/006962.html

I'll post here again in a few days with a summary / recommendation on how to proceed.

Still, opinions welcome of course here in the meantime what you'd prefer to have for Astropy core!

Should we remove v2 milestone for this?

@cdeil

@cdeil - Your requires_dependency is definitely nicer than what we currently do, would you be interested in contributing it to astropy? I guess it doesn't need much more than a copy-paste from gammapy. It would be definitely an improvement at least until you (or maybe someone else) have enough spare time to do the separate pytest pluggin way.

Before contributing it to astropy, what about trying to contribute it to pytest?

Well, what about having it in astropy until it comes out either in pytest or a pytest plugin?

Re-milestone to 3.1?

Future?

@drdavella

BTW, I have personally lost interest in this, so I would say the astropy testing maintainer @drdavella should decide whether this is a priority or just close.

cc: @astrofrog @eteq

@astrofrog

It's maybe not a priority, but it's definitely worth keeping open for consideration, imho.

I agree with other comments above that a declarative approach would be useful, and I also agree with @astrofrog that it should potentially be contributed to pytest directly.

I think we use that now where we have to. If we missed a few places, more specific issues should be opened instead.

Navigation Menu

Repository navigation

Consider using pytest.importorskip('dependency') in testing #5543

1 remaining item

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Consider using pytest.importorskip('dependency') in testing #5543

Description

Activity

astrofrog commented on Dec 2, 2016

taldcroft commented on Dec 2, 2016

bsipocz commented on Dec 2, 2016

cdeil commented on Dec 7, 2016

bsipocz commented on Dec 7, 2016

Cadair commented on Dec 7, 2016

saimn commented on Dec 7, 2016

cdeil commented on Dec 7, 2016

cdeil commented on Dec 7, 2016

bsipocz commented on Dec 7, 2016

cdeil commented on Dec 7, 2016

pllim commented on Jun 15, 2017

1 remaining item

bsipocz commented on Aug 1, 2017

astrofrog commented on Aug 1, 2017

bsipocz commented on Aug 1, 2017

pllim commented on Jan 23, 2018

taldcroft commented on Jan 23, 2018

taldcroft commented on Jan 23, 2018

drdavella commented on Jan 23, 2018

pllim commented on Feb 14, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions