pandas: [Good first issue] TST: Disallow bare pytest.raises
End users rely on error messages for their debugging purposes. Thus, it is important that we make sure that the correct error messages are surfaced depending on the error triggered.
The core idea is to convert this:
with pytest.raises(klass):
# Some code that raise an error
To this:
with pytest.raises(klass, match=msg):
# Some code that raise an error
You can read more about pytest.raises here.
Side note:
In case that the raised error message is an external error message (meaning that’s not pandas specific), you should use the external_error_raised instead of pytest.raises.
the usage of external_error_raised is exactly like pytest.raises the only difference is that you don’t pass in the match argument.
For example:
import pandas._testing as tm
def test_foo():
with tm.external_error_raised(ValueError):
raise ValueError("foo")
Keynotes:
- Don’t forget to link this issue in your PR, paste this
https://github.com/pandas-dev/pandas/issues/30999
in your PR.
-
Please comment what you are planning to work on, so we won’t do double the work (no need to mention me, you can just declare what you are planning to work on, just remember to check if something is already taken).
-
If a file/files that should be marked as “done” (as if there is no more work to do), isn’t marked as “done”, please comment letting me know about that (And mentioning me by putting
@MomIsBestFriendat the comment’s body, so I’ll know where to look).
To generate the full list yourself, you can add:
- id: unwanted-patterns-bare-pytest-raises
name: Check for use of bare use of pytest raises
language: python
entry: python scripts/validate_unwanted_patterns.py --validation-type="bare_pytest_raises"
types: [python]
files: ^pandas/tests/
exclude: ^pandas/tests/extension
to .pre-commit-config.yaml in the - repo: local section, and then run
pre-commit run unwanted-patterns-bare-pytest-raises --all-files.
The current list is:
- pandas/tests/arrays/boolean/test_arithmetic.py
- pandas/tests/computation/test_compat.py
- pandas/tests/dtypes/test_inference.py
- pandas/tests/indexes/multi/test_indexing.py
- pandas/tests/indexes/multi/test_setops.py
- pandas/tests/indexes/period/test_indexing.py
- pandas/tests/indexes/test_common.py
- pandas/tests/indexes/test_numpy_compat.py
- pandas/tests/indexing/multiindex/test_partial.py
- pandas/tests/indexing/test_coercion.py
- pandas/tests/io/test_sql.py
- pandas/tests/libs/test_hashtable.py
- pandas/tests/reductions/test_reductions.py
- pandas/tests/reductions/test_stat_reductions.py
- pandas/tests/resample/test_resampler_grouper.py
- pandas/tests/reshape/test_get_dummies.py
- pandas/tests/reshape/test_union_categoricals.py
- pandas/tests/series/apply/test_series_apply.py
- pandas/tests/series/test_ufunc.py
- pandas/tests/window/moments/test_moments_ewm.py
- pandas/tests/window/test_apply.py
NOTE:
The list may change as files are moved/renamed constantly.
Took pretty much everything from #23922, that was originally opened by @gfyoung.
About this issue
- Original URL
- State: closed
- Created 4 years ago
- Comments: 84 (79 by maintainers)
Commits related to this issue
- Issue ##30999, Added match = msg as a second parameter in applicable lines. — committed to DylanBrdt/pandas by DylanBrdt 4 years ago
- Implemented "external_error_raised" @gfyoung wrote this function here https://github.com/pandas-dev/pandas/issues/30999#issuecomment-575426086 so we can ignore error messages without breaking fututre... — committed to ShaharNaveh/pandas by deleted user 4 years ago
- TST: Implement external error raised helper function. (#31130) @gfyoung wrote this function here https://github.com/pandas-dev/pandas/issues/30999#issuecomment-575426086 so we can ignore error mess... — committed to pandas-dev/pandas by ShaharNaveh 4 years ago
- TST: add messages to pytest.raises (#30999) — committed to proost/pandas by proost 4 years ago
- TST: add messages to pytest.raises (#30999) — committed to proost/pandas by proost 4 years ago
- TST: add messages to pytest.raises (#30999) (#33650) — committed to pandas-dev/pandas by proost 4 years ago
- TST: add messages to pytest.raises (#30999) (#33650) — committed to CloseChoice/pandas by proost 4 years ago
- TST: Black formatting update (#30999) — committed to Dom-L-G/pandas by Dom-L-G 4 years ago
- TST: Added message to bare pytest.raises in test_join_multi_levels (#30999) — committed to Dom-L-G/pandas by Dom-L-G 4 years ago
- TST: Added message to bare pytest.raises in test_join_multi_levels (#30999) (#33806) — committed to pandas-dev/pandas by Dom-L-G 4 years ago
- TST: add messages to pytest.raises (#30999) (#33650) — committed to rhshadrach/pandas by proost 4 years ago
- TST: Added message to bare pytest.raises in test_join_multi_levels (#30999) (#33806) — committed to rhshadrach/pandas by Dom-L-G 4 years ago
- (#30999) tests/window/moments/test_moments_rolling.py: msg + match added — committed to boweyism/pandas by boweyism 4 years ago
- (#30999) tests/window/moments/test_moments_rolling.py: fixed invalid escape sequence \[ — committed to boweyism/pandas by boweyism 4 years ago
- TST (#30999) tests/window/test_dtypes.py: msg + match — committed to boweyism/pandas by boweyism 4 years ago
- TST (#30999) tests/window/test_ewm.py: msg + match — committed to boweyism/pandas by boweyism 4 years ago
- TST (#30999) tests/window/test_expanding.py: msg + match — committed to boweyism/pandas by boweyism 4 years ago
- TST (#30999) tests/window/test_timeseries_windows.py: msg + match — committed to boweyism/pandas by boweyism 4 years ago
- TST: pandas/test/window/moments/test_moments_ewm.py: msg and match arguments (#30999) (#30999) tests/window/moments/test_moments_rolling.py: msg + match added (#30999) tests/window/moments/test_mome... — committed to boweyism/pandas by boweyism 4 years ago
- black formatting (#30999) — committed to boweyism/pandas by boweyism 4 years ago
Thanks, I’ve updated the instructions, no need to change the script
We will add it to
.pre-commit-config.yamlonce all the errors it raises are fixed, yesNo, it’s related to #37379 (which is when we moved this script over to pre-commit, hence it was no longer necessary for it to run on directories)
@MomIsBestFriend this one is done already but isn’t marked as done: pandas/tests/arithmetic/test_numeric.py
UPDATE
@MomIsBestFriend these too: pandas/tests/arithmetic/test_period.py pandas/tests/arrays/test_integer.py pandas/tests/arrays/test_period.py
pandas._testing#31072 contains the one missing match in
stata.pyI can take care of these: @MomIsBestFriend
pandas/tests/io/test_html.py pandas/tests/io/test_parquet.py pandas/tests/io/test_sql.py pandas/tests/io/test_stata.py pandas/tests/plotting/test_backend.py pandas/tests/plotting/test_boxplot_method.py pandas/tests/plotting/test_frame.py pandas/tests/plotting/test_hist_method.py pandas/tests/plotting/test_series.py pandas/tests/reductions/test_reductions.py
@MomIsBestFriend I will help with : pandas/tests/base/test_constructors.py pandas/tests/base/test_ops.py
@gfyoung the list wasn’t generated by
grep -r -e "pytest.raises([a-zA-Z]*)" pandas/tests -lin fact, it was generated by the script in #30755 (a validation type calledbare_pytest_raises), I will put instructions at the issue body, once it gets merged 😄@gdex1 I hope this will help you 😃
(The numbers represents line number)