TST: return pytest MarkDecorator from td.skip_if_no #26735
Conversation
simonjayhawkins
added
the
Testing
pandas/util/_test_decorators.py
Outdated
| @@ -99,36 +99,40 @@ def _skip_if_no_scipy(): | |||
|
|
|||
| def skip_if_no(package, min_version=None): | |||
There was a problem hiding this comment.
can you add annotations (return value in particular)
There was a problem hiding this comment.
can you add annotations (return value in particular)
done
Codecov Report
@@ Coverage Diff @@
## master #26735 +/- ##
==========================================
- Coverage 91.78% 91.77% -0.01%
==========================================
Files 174 174
Lines 50703 50701 -2
==========================================
- Hits 46538 46532 -6
- Misses 4165 4169 +4
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #26735 +/- ##
==========================================
- Coverage 91.78% 91.77% -0.01%
==========================================
Files 174 174
Lines 50703 50703
==========================================
- Hits 46538 46534 -4
- Misses 4165 4169 +4
Continue to review full report at Codecov.
|
| @@ -97,38 +101,42 @@ def _skip_if_no_scipy(): | |||
| safe_import('scipy.signal')) | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
why wouldn’t you make this MarkDecorator)
There was a problem hiding this comment.
Nice change with much cleaner syntax going forward. Thanks @simonjayhawkins
| not td.safe_import('lxml'), reason='No bs4')), | ||
| pytest.param('lxml', marks=pytest.mark.skipif( | ||
| not td.safe_import('lxml'), reason='No lxml'))], scope="class") | ||
| pytest.param('bs4', marks=td.skip_if_no('lxml')), |
There was a problem hiding this comment.
There are some of these in test_excel as well. Not sure if you meant to do all in this PR or not but if so worth a double check of other modules
There was a problem hiding this comment.
left them intentionally to avoid conflicts with your PRs.
have checked other modules, no other use of safe_import except one case of a test decorator where I think skip_if_no could be used directly. (change that in this PR?)
pandas/pandas/tests/io/test_gcs.py
Lines 67 to 69 in ee6b131
and test_safe_import.
|
thanks, possibly worthile to make an issue to identify places where we can fix this in the code (or just do it if not that hard) |
skip_if_noreturns a decorator that applies a pytest mark.pytest marks are decorators and can be applied directly to test functions.
This PR changes
skip_if_noto return just the pytest mark.skip_if_nocan then be used in (pytest.param(... , marks=) situations. eg. pandas/tests/io/test_html.pycould also be used in pandas/tests/io/test_excel.py but not done here to avoid overlapping PRs.
safe_importcould potentially be renamed_safe_importsince would not be needed directly in tests.cc @WillAyd