TST: Disallow bare pytest.raises #23922

gfyoung · 2018-11-26T09:50:04Z

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.

~~I would like to propose that we disallow the following:~~
I think we should convert the following:

with pytest.raises(klass):
   # Your code here.

And in the spirit of tm.assert_raises_regex, we should ensure that all tests have the following:

with pytest.raises(klass, match=msg):
    # Your code here.

There are likely a few corner cases that would need to be ironed out, but I think the net benefit of converting this format would be much better for the robustness of our tests.

If we are going to focus on error message quality, then let's test them consistently! 🙂

Want to open to discussion first to see if we are on board with this. If we are, then this issue will be converted to one for doing said conversions and then (maybe) enforcing in our linter.

xref #16521
xref #23550
xref #23853

===============================================================

Running this command:

grep -r -e "pytest.raises([a-zA-Z]*)" pandas/tests -l

yields the following list:

pandas/tests/api/test_types.py
pandas/tests/arithmetic/test_datetime64.py
pandas/tests/arithmetic/test_numeric.py
pandas/tests/arithmetic/test_object.py
pandas/tests/arithmetic/test_period.py
pandas/tests/arithmetic/test_timedelta64.py
pandas/tests/arrays/categorical/test_algos.py
pandas/tests/arrays/categorical/test_analytics.py
pandas/tests/arrays/categorical/test_api.py
pandas/tests/arrays/categorical/test_constructors.py
pandas/tests/arrays/categorical/test_indexing.py
pandas/tests/arrays/categorical/test_operators.py
pandas/tests/arrays/interval/test_ops.py
pandas/tests/arrays/sparse/test_array.py
pandas/tests/arrays/sparse/test_dtype.py
pandas/tests/arrays/sparse/test_libsparse.py
pandas/tests/arrays/test_array.py
pandas/tests/arrays/test_datetimelike.py
pandas/tests/arrays/test_integer.py
pandas/tests/computation/test_compat.py
pandas/tests/computation/test_eval.py
pandas/tests/dtypes/test_cast.py
pandas/tests/dtypes/test_dtypes.py
pandas/tests/extension/base/getitem.py
pandas/tests/extension/base/ops.py
pandas/tests/extension/base/reduce.py
pandas/tests/extension/base/setitem.py
pandas/tests/extension/decimal/test_decimal.py
pandas/tests/extension/json/test_json.py
pandas/tests/extension/test_categorical.py
pandas/tests/extension/test_integer.py
pandas/tests/frame/test_analytics.py
pandas/tests/frame/test_apply.py
pandas/tests/frame/test_arithmetic.py
pandas/tests/frame/test_axis_select_reindex.py
pandas/tests/frame/test_constructors.py
pandas/tests/frame/test_convert_to.py
pandas/tests/frame/test_dtypes.py
pandas/tests/frame/test_duplicates.py
pandas/tests/frame/test_indexing.py
pandas/tests/frame/test_missing.py
pandas/tests/frame/test_mutate_columns.py
pandas/tests/frame/test_operators.py
pandas/tests/frame/test_quantile.py
pandas/tests/frame/test_query_eval.py
pandas/tests/frame/test_reshape.py
pandas/tests/frame/test_sorting.py
pandas/tests/frame/test_timeseries.py
pandas/tests/generic/test_frame.py
pandas/tests/generic/test_generic.py
pandas/tests/generic/test_series.py
pandas/tests/groupby/test_categorical.py
pandas/tests/groupby/test_groupby.py
pandas/tests/groupby/test_timegrouper.py
pandas/tests/indexes/common.py
pandas/tests/indexes/datetimelike.py
pandas/tests/indexes/datetimes/test_arithmetic.py
pandas/tests/indexes/datetimes/test_astype.py
pandas/tests/indexes/datetimes/test_construction.py
pandas/tests/indexes/datetimes/test_date_range.py
pandas/tests/indexes/datetimes/test_indexing.py
pandas/tests/indexes/datetimes/test_timezones.py
pandas/tests/indexes/datetimes/test_tools.py
pandas/tests/indexes/interval/test_astype.py
pandas/tests/indexes/interval/test_interval.py
pandas/tests/indexes/interval/test_interval_new.py
pandas/tests/indexes/interval/test_interval_tree.py
pandas/tests/indexes/multi/test_analytics.py
pandas/tests/indexes/multi/test_compat.py
pandas/tests/indexes/multi/test_constructor.py
pandas/tests/indexes/multi/test_drop.py
pandas/tests/indexes/multi/test_duplicates.py
pandas/tests/indexes/multi/test_get_set.py
pandas/tests/indexes/multi/test_indexing.py
pandas/tests/indexes/multi/test_integrity.py
pandas/tests/indexes/multi/test_monotonic.py
pandas/tests/indexes/multi/test_partial_indexing.py
pandas/tests/indexes/multi/test_sorting.py
pandas/tests/indexes/period/test_arithmetic.py
pandas/tests/indexes/period/test_construction.py
pandas/tests/indexes/period/test_indexing.py
pandas/tests/indexes/period/test_partial_slicing.py
pandas/tests/indexes/period/test_period.py
pandas/tests/indexes/test_base.py
pandas/tests/indexes/test_category.py
pandas/tests/indexes/test_common.py
pandas/tests/indexes/test_numeric.py
pandas/tests/indexes/test_range.py
pandas/tests/indexes/timedeltas/test_arithmetic.py
pandas/tests/indexes/timedeltas/test_construction.py
pandas/tests/indexes/timedeltas/test_indexing.py
pandas/tests/indexing/interval/test_interval.py
pandas/tests/indexing/interval/test_interval_new.py
pandas/tests/indexing/multiindex/test_setitem.py
pandas/tests/indexing/multiindex/test_slice.py
pandas/tests/indexing/test_categorical.py
pandas/tests/indexing/test_indexing.py
pandas/tests/indexing/test_loc.py
pandas/tests/indexing/test_panel.py
pandas/tests/indexing/test_partial.py
pandas/tests/indexing/test_scalar.py
pandas/tests/internals/test_internals.py
pandas/tests/io/formats/test_format.py
pandas/tests/io/formats/test_style.py
pandas/tests/io/formats/test_to_latex.py
pandas/tests/io/json/test_json_table_schema.py
pandas/tests/io/json/test_normalize.py
pandas/tests/io/json/test_pandas.py
pandas/tests/io/json/test_ujson.py
pandas/tests/io/parser/test_network.py
pandas/tests/io/parser/test_python_parser_only.py
pandas/tests/io/sas/test_sas7bdat.py
pandas/tests/io/test_clipboard.py
pandas/tests/io/test_common.py
pandas/tests/io/test_excel.py
pandas/tests/io/test_feather.py
pandas/tests/io/test_gcs.py
pandas/tests/io/test_html.py
pandas/tests/io/test_parquet.py
pandas/tests/io/test_pytables.py
pandas/tests/io/test_sql.py
pandas/tests/io/test_stata.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
pandas/tests/reductions/test_stat_reductions.py
pandas/tests/resample/test_base.py
pandas/tests/resample/test_period_index.py
pandas/tests/resample/test_resample_api.py
pandas/tests/reshape/merge/test_join.py
pandas/tests/reshape/merge/test_merge.py
pandas/tests/reshape/merge/test_merge_asof.py
pandas/tests/reshape/merge/test_multi.py
pandas/tests/reshape/test_concat.py
pandas/tests/reshape/test_melt.py
pandas/tests/reshape/test_pivot.py
pandas/tests/reshape/test_reshape.py
pandas/tests/reshape/test_union_categoricals.py
pandas/tests/scalar/interval/test_interval.py
pandas/tests/scalar/period/test_asfreq.py
pandas/tests/scalar/period/test_period.py
pandas/tests/scalar/timedelta/test_arithmetic.py
pandas/tests/scalar/timedelta/test_construction.py
pandas/tests/scalar/timedelta/test_timedelta.py
pandas/tests/scalar/timestamp/test_comparisons.py
pandas/tests/scalar/timestamp/test_timestamp.py
pandas/tests/scalar/timestamp/test_timezones.py
pandas/tests/scalar/timestamp/test_unary_ops.py
pandas/tests/series/indexing/test_alter_index.py
pandas/tests/series/indexing/test_boolean.py
pandas/tests/series/indexing/test_datetime.py
pandas/tests/series/indexing/test_indexing.py
pandas/tests/series/indexing/test_numeric.py
pandas/tests/series/test_alter_axes.py
pandas/tests/series/test_analytics.py
pandas/tests/series/test_api.py
pandas/tests/series/test_apply.py
pandas/tests/series/test_arithmetic.py
pandas/tests/series/test_asof.py
pandas/tests/series/test_dtypes.py
pandas/tests/series/test_missing.py
pandas/tests/series/test_operators.py
pandas/tests/series/test_replace.py
pandas/tests/series/test_timeseries.py
pandas/tests/series/test_timezones.py
pandas/tests/sparse/frame/test_frame.py
pandas/tests/sparse/test_indexing.py
pandas/tests/test_base.py
pandas/tests/test_common.py
pandas/tests/test_errors.py
pandas/tests/test_lib.py
pandas/tests/test_nanops.py
pandas/tests/test_panel.py
pandas/tests/test_take.py
pandas/tests/test_window.py
pandas/tests/tools/test_numeric.py
pandas/tests/tseries/offsets/test_offsets.py
pandas/tests/tseries/offsets/test_ticks.py
pandas/tests/tseries/test_frequencies.py
pandas/tests/tseries/test_holiday.py
pandas/tests/tslibs/test_array_to_datetime.py
pandas/tests/tslibs/test_liboffsets.py
pandas/tests/tslibs/test_timedeltas.py
pandas/tests/tslibs/test_timezones.py

The text was updated successfully, but these errors were encountered:

datapythonista · 2018-11-26T10:00:45Z

Just to provide some additional info, I did couple of greps, and I see that:

We have 3882 calls to pytest.raises
1376 already have a match (in the same line as pytest,raises), leaving around 2500 cases to change
301 cases have arguments of pytest.raises in multiple lines, and that could make the linting tricky

gfyoung · 2018-11-26T10:03:30Z

301 cases have arguments of pytest.raises in multiple lines, and that could make the linting tricky

That's good to know. We can probably test for the existence of a closing parenthesis (in some way shape or form) is my first thought.

jorisvandenbossche · 2018-11-26T14:14:46Z

Personally, I am not sure enforcing it here is needed. This will be a huge amount of work, which IMO would be better spent actually improving error messages.
And which does not mean that we could pay more attention to it for new code, that we try to more consistently check the message.

gfyoung · 2018-11-26T17:50:46Z

This will be a huge amount of work

@jorisvandenbossche : I'm pretty sure we have plenty of people who would be happy to contribute to the effort, so I'm not concerned about that. 😉

gfyoung · 2018-11-26T21:00:20Z

I'm okay with putting aside enforcement, but if checking error messages is a thing we should be focusing on in the future, I think converting these bare pytest.raises would still be a good thing for us (and by us, I mean contributors) to do here.

gfyoung · 2018-11-26T21:08:46Z

@datapythonista : I'm not sure what grep's you ran, but I ran one that finds 179 files that contain bare pytest.raises as follows:

grep -r -e "pytest.raises([a-zA-Z]*)" pandas/tests -l

pandas/tests/arithmetic/test_datetime64.py
pandas/tests/arithmetic/test_numeric.py
pandas/tests/arithmetic/test_object.py
pandas/tests/arithmetic/test_period.py
pandas/tests/arithmetic/test_timedelta64.py
pandas/tests/arrays/categorical/test_algos.py
pandas/tests/arrays/categorical/test_analytics.py
pandas/tests/arrays/categorical/test_api.py
pandas/tests/arrays/categorical/test_constructors.py
pandas/tests/arrays/categorical/test_operators.py
pandas/tests/arrays/interval/test_ops.py
pandas/tests/arrays/sparse/test_array.py
pandas/tests/arrays/sparse/test_dtype.py
pandas/tests/arrays/sparse/test_libsparse.py
pandas/tests/arrays/test_datetimelike.py
pandas/tests/arrays/test_integer.py
pandas/tests/computation/test_compat.py
pandas/tests/computation/test_eval.py
pandas/tests/dtypes/test_cast.py
pandas/tests/dtypes/test_dtypes.py
pandas/tests/extension/base/getitem.py
pandas/tests/extension/base/ops.py
pandas/tests/extension/base/reduce.py
pandas/tests/extension/decimal/test_decimal.py
pandas/tests/extension/json/test_json.py
pandas/tests/extension/test_categorical.py
pandas/tests/extension/test_integer.py
pandas/tests/frame/test_analytics.py
pandas/tests/frame/test_apply.py
pandas/tests/frame/test_arithmetic.py
pandas/tests/frame/test_axis_select_reindex.py
pandas/tests/frame/test_constructors.py
pandas/tests/frame/test_convert_to.py
pandas/tests/frame/test_dtypes.py
pandas/tests/frame/test_duplicates.py
pandas/tests/frame/test_indexing.py
pandas/tests/frame/test_missing.py
pandas/tests/frame/test_mutate_columns.py
pandas/tests/frame/test_operators.py
pandas/tests/frame/test_query_eval.py
pandas/tests/frame/test_reshape.py
pandas/tests/frame/test_sorting.py
pandas/tests/frame/test_timeseries.py
pandas/tests/generic/test_generic.py
pandas/tests/generic/test_series.py
pandas/tests/groupby/test_groupby.py
pandas/tests/groupby/test_timegrouper.py
pandas/tests/indexes/common.py
pandas/tests/indexes/datetimelike.py
pandas/tests/indexes/datetimes/test_arithmetic.py
pandas/tests/indexes/datetimes/test_construction.py
pandas/tests/indexes/datetimes/test_date_range.py
pandas/tests/indexes/datetimes/test_indexing.py
pandas/tests/indexes/datetimes/test_timezones.py
pandas/tests/indexes/datetimes/test_tools.py
pandas/tests/indexes/interval/test_astype.py
pandas/tests/indexes/interval/test_interval.py
pandas/tests/indexes/interval/test_interval_new.py
pandas/tests/indexes/interval/test_interval_tree.py
pandas/tests/indexes/multi/test_analytics.py
pandas/tests/indexes/multi/test_compat.py
pandas/tests/indexes/multi/test_constructor.py
pandas/tests/indexes/multi/test_drop.py
pandas/tests/indexes/multi/test_duplicates.py
pandas/tests/indexes/multi/test_get_set.py
pandas/tests/indexes/multi/test_indexing.py
pandas/tests/indexes/multi/test_integrity.py
pandas/tests/indexes/multi/test_monotonic.py
pandas/tests/indexes/multi/test_partial_indexing.py
pandas/tests/indexes/multi/test_sorting.py
pandas/tests/indexes/period/test_arithmetic.py
pandas/tests/indexes/period/test_construction.py
pandas/tests/indexes/period/test_indexing.py
pandas/tests/indexes/period/test_partial_slicing.py
pandas/tests/indexes/period/test_period.py
pandas/tests/indexes/test_base.py
pandas/tests/indexes/test_category.py
pandas/tests/indexes/test_common.py
pandas/tests/indexes/test_numeric.py
pandas/tests/indexes/test_range.py
pandas/tests/indexes/timedeltas/test_arithmetic.py
pandas/tests/indexes/timedeltas/test_construction.py
pandas/tests/indexes/timedeltas/test_indexing.py
pandas/tests/indexes/timedeltas/test_timedelta_range.py
pandas/tests/indexing/interval/test_interval.py
pandas/tests/indexing/interval/test_interval_new.py
pandas/tests/indexing/test_categorical.py
pandas/tests/indexing/test_indexing.py
pandas/tests/indexing/test_loc.py
pandas/tests/indexing/test_multiindex.py
pandas/tests/indexing/test_panel.py
pandas/tests/indexing/test_scalar.py
pandas/tests/internals/test_internals.py
pandas/tests/io/formats/test_format.py
pandas/tests/io/formats/test_style.py
pandas/tests/io/formats/test_to_latex.py
pandas/tests/io/json/test_json_table_schema.py
pandas/tests/io/json/test_normalize.py
pandas/tests/io/json/test_pandas.py
pandas/tests/io/json/test_ujson.py
pandas/tests/io/parser/test_network.py
pandas/tests/io/parser/test_python_parser_only.py
pandas/tests/io/parser/test_read_fwf.py
pandas/tests/io/sas/test_sas7bdat.py
pandas/tests/io/test_clipboard.py
pandas/tests/io/test_common.py
pandas/tests/io/test_excel.py
pandas/tests/io/test_feather.py
pandas/tests/io/test_gcs.py
pandas/tests/io/test_html.py
pandas/tests/io/test_parquet.py
pandas/tests/io/test_pytables.py
pandas/tests/io/test_sql.py
pandas/tests/io/test_stata.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/resample/test_base.py
pandas/tests/resample/test_datetime_index.py
pandas/tests/resample/test_period_index.py
pandas/tests/resample/test_resample_api.py
pandas/tests/reshape/merge/test_join.py
pandas/tests/reshape/merge/test_merge.py
pandas/tests/reshape/merge/test_merge_asof.py
pandas/tests/reshape/test_concat.py
pandas/tests/reshape/test_melt.py
pandas/tests/reshape/test_pivot.py
pandas/tests/reshape/test_reshape.py
pandas/tests/reshape/test_tile.py
pandas/tests/reshape/test_union_categoricals.py
pandas/tests/scalar/interval/test_interval.py
pandas/tests/scalar/period/test_asfreq.py
pandas/tests/scalar/period/test_period.py
pandas/tests/scalar/test_nat.py
pandas/tests/scalar/timedelta/test_arithmetic.py
pandas/tests/scalar/timedelta/test_construction.py
pandas/tests/scalar/timedelta/test_timedelta.py
pandas/tests/scalar/timestamp/test_comparisons.py
pandas/tests/scalar/timestamp/test_timestamp.py
pandas/tests/scalar/timestamp/test_timezones.py
pandas/tests/scalar/timestamp/test_unary_ops.py
pandas/tests/series/indexing/test_alter_index.py
pandas/tests/series/indexing/test_datetime.py
pandas/tests/series/indexing/test_indexing.py
pandas/tests/series/test_alter_axes.py
pandas/tests/series/test_analytics.py
pandas/tests/series/test_api.py
pandas/tests/series/test_apply.py
pandas/tests/series/test_arithmetic.py
pandas/tests/series/test_asof.py
pandas/tests/series/test_dtypes.py
pandas/tests/series/test_missing.py
pandas/tests/series/test_operators.py
pandas/tests/series/test_replace.py
pandas/tests/series/test_timeseries.py
pandas/tests/series/test_timezones.py
pandas/tests/sparse/frame/test_frame.py
pandas/tests/sparse/test_indexing.py
pandas/tests/test_base.py
pandas/tests/test_common.py
pandas/tests/test_errors.py
pandas/tests/test_lib.py
pandas/tests/test_nanops.py
pandas/tests/test_panel.py
pandas/tests/test_take.py
pandas/tests/test_window.py
pandas/tests/tools/test_numeric.py
pandas/tests/tseries/offsets/test_offsets.py
pandas/tests/tseries/offsets/test_ticks.py
pandas/tests/tseries/test_frequencies.py
pandas/tests/tseries/test_holiday.py
pandas/tests/tslibs/test_array_to_datetime.py
pandas/tests/tslibs/test_liboffsets.py
pandas/tests/tslibs/test_timedeltas.py
pandas/tests/tslibs/test_timezones.py
pandas/tests/util/test_hashing.py
pandas/tests/util/test_testing.py
pandas/tests/util/test_util.py

gfyoung · 2018-11-26T21:09:24Z

For anyone who is interested in contributing, working on any one of the files listed above would be great!

WillAyd · 2018-11-26T23:08:05Z

Personally, I am not sure enforcing it here is needed. This will be a huge amount of work, which IMO would be better spent actually improving error messages.

I agree with @jorisvandenbossche here as I'm not sure how much we stand to gain out of this. There will be a lot of instances where incremental value add is nil or questionable.

@jorisvandenbossche : I'm pretty sure we have plenty of people who would be happy to contribute to the effort, so I'm not concerned about that

We already have a decent amount of PRs concerning code style and docstrings atm and the PR queue is larger than it's even been. Is this something we feel needs to be addressed now or can we come back to it once some of the noise around docstrings settles down?

gfyoung · 2018-11-26T23:22:39Z

There will be a lot of instances where incremental value add is nil or questionable.

I won't rule out corner cases, but I'm pretty certain there are more instances where there will be value added. We don't have to push enforcing, but let's at least take a look and convert these. And if they aren't giving relevant error messages, then that's a good sign to change them.

Is this something we feel needs to be addressed now or can we come back to it once some of the noise around docstrings settles down?

Hardly. Otherwise, I would have tagged this for a release version. This is just another way for people to contribute to pandas if they so choose.

jbrockmendel · 2018-11-27T00:57:36Z

I agree with everyone. This is a worthwhile goal, not an urgent goal, and a good way for newcomers to make a real contribution while learning the code.

The linting part might be easier if we required the contextmanager usage for pytest.raises. Then we should be able to identify the close-paren by the colon and newline.

To find non-context usages: re.search("(?<!with )pytest.raises", code)
To find match-less:

pattern = r"pytest.raises\((?!.*match=).*\):\n"
re.search(pattern, code, flags=re.M)

I'm sure the latter has corner cases I'm missing.

datapythonista · 2018-11-27T10:15:28Z

@gfyoung the numbers I shared weren't number of files, but the times that pytest.raises happen.

What I'd do is create an issue to add the match to one of the files, label it as "good first issue", see how it goes (how many cases are not obvious, or it doesn't make sense to add the match, how time consuming is to make the changes and to review...). And afterwards we can continue the discussion with more information.

Does that make sense?

Follow-up to pandas-devgh-23999. xref pandas-devgh-23922.

Follow-up to gh-23999. xref gh-23922.

bashtage · 2018-12-04T08:24:51Z

I don't think your regex is correct. For example, it found

            pytest.raises(
                KeyError, lambda: written_and_read_again['index_not_written'])

in test_stata

Should probably looks for pytest.raises()

jbrockmendel · 2018-12-18T14:52:08Z

Instead of using regex, what if we patched pytest.raises to disallow the unwanted uses? We could put the patch inside of module setup/teardown functions

gfyoung · 2018-12-18T18:22:28Z

what if we patched pytest.raises to disallow the unwanted uses

@jbrockmendel : By unwanted uses, what do you mean? And patch how exactly (not sure I follow you here 100%)?

jbrockmendel · 2018-12-18T19:27:44Z

By unwanted uses, what do you mean? And patch how exactly (not sure I follow you here 100%)?

Something along the lines of:

_raises = pytest.raises

def raises(*args, **kwargs):
    if kwargs.get("match", None) is None:
        raise LintingComplaint
    return _raises(*args, **kwargs)

def setup_module():
    pytest.raises = raises

def teardown_module():
    pytest.raises = _raises

jbrockmendel · 2018-12-18T19:29:02Z

I'm not necessarily saying this is a good idea, just that it avoids some of the pitfalls of grep-based linting.

xref gh-23922

mike-cramblett · 2019-01-23T20:36:36Z

@gfyoung
Hi there! I'm interested in working on adding the match parameters to the pytest.raises calls in those test files as my first issue. It seems like a great way to familiarize myself with testing in pandas. Is there enough consensus here for me to start? And if so should I take the first file or a chunk of them from the top of that grep list? And thanks!

gfyoung · 2019-01-23T21:13:26Z

@mike-cramblett : That's fantastic! Let me answer your questions:

Is there enough consensus here for me to start?

Yes.

And if so should I take the first file or a chunk of them from the top of that grep list?

I would suggest doing one file first and seeing how that goes. Some are trickier than others. In addition, that list is a little dated (I need to run the grep again to update the list). That being said, you can run it locally in the directory of your pandas repository to get an updated list yourself.

mike-cramblett · 2019-01-23T21:33:59Z

@gfyoung

Thanks for the info and encouragement! Just reran that grep and got 162 files. I was figuring I'd start with tests/arithmetic/test_datetime64.py since it's first on the list. But it looks like that file has 94 pytest.raises calls to fix. I'm not that daunted by doing all that, since it seems they're mostly similar TypeErrors. But I'd also be open to starting on a simpler file if you have one in mind. Thanks again!

jbrockmendel · 2019-06-11T15:32:50Z

@gfyoung are we close to the goal line here?

jbrockmendel · 2019-06-11T15:32:59Z

wrong button

simonjayhawkins · 2019-06-11T15:57:27Z

@gfyoung are we close to the goal line here?

at least 1167 still to do. quite time consuming. so not close.

WillAyd · 2019-06-11T15:59:43Z

Question for the team - do we see any improvements for our current approach? Half of me continues to like this change as it can catch some subtle differences, but the other half of me thinks this causes unnecessary noise with CI (#26726 being an example)

simonjayhawkins · 2019-06-11T16:11:30Z

the CI breakages are an additional burden.

but I do think that it helps identify inconsistencies in user-facing exception messages.

WillAyd · 2019-06-11T16:43:25Z

So two things I've thought about:

We set a pseudo-standard to only pick 2-3 words max from the error message
We only enforce error messages for things raised internally

With option 1 I think we should really avoid complicated patterns like this:

pandas/pandas/tests/io/parser/test_na_values.py

Line 433 in 370054e

msg = ("(Bool column has NA values in column [0a])|"

Where we aren't actually managing expectations around what the user sees as much as implicity doing some versioning to make CI happy

For option 2 I just think failing because of a message change from an external dependency generates a lot of busy work for something we don't control anyway. Not sure how enforceable this is

simonjayhawkins · 2019-06-11T16:51:21Z

So two things I've thought about:

We set a pseudo-standard to only pick 2-3 words max from the error message

We only enforce error messages for things raised internally

makes sense. maybe explicit checks for 2. and use 1 for messages raised by other packages.

that would solve the CI issues and still help improve user-facing error messages.

gfyoung · 2019-06-11T22:08:03Z

I second @simonjayhawkins suggestion as well

jreback · 2019-06-12T00:19:23Z

i am ok with limiting how much we check in our error messages (option 1)
option 2 are generally good at catching changing dependencies and don’t happen too often - so wouldn’t change anything here

CI failures are a canary and so good to have - even if occasionally are a false positive

simonjayhawkins · 2019-06-12T12:27:52Z

A potential issue i can foresee with explicitly checking error messages from 3rd party packages is where the pandas test suite is used to validate environments and distributions.

After a pandas release, the test suite could fail purely due to subtle changes in the messages raised by updated dependencies.

jbrockmendel · 2019-10-16T22:28:49Z

@gfyoung looks like we now have a code check for this. is this closed by #27354?

gfyoung · 2019-10-16T23:07:39Z

@jbrockmendel : I think there is still a fair distance to go here.

looks like we now have a code check for this

Where in the code_checks script do we have a check for this?

Here is the updated list of files that still have bare pytest.raises :

pandas/pandas/tests/io/test_html.py
pandas/pandas/tests/io/test_parquet.py
pandas/pandas/tests/io/test_sql.py
pandas/pandas/tests/io/test_stata.py
pandas/pandas/tests/plotting/test_backend.py
pandas/pandas/tests/plotting/test_boxplot_method.py
pandas/pandas/tests/plotting/test_frame.py
pandas/pandas/tests/plotting/test_hist_method.py
pandas/pandas/tests/plotting/test_series.py
pandas/pandas/tests/reductions/test_reductions.py
pandas/pandas/tests/reductions/test_stat_reductions.py
pandas/pandas/tests/reshape/merge/test_merge_asof.py
pandas/pandas/tests/reshape/merge/test_multi.py
pandas/pandas/tests/reshape/test_reshape.py
pandas/pandas/tests/reshape/test_union_categoricals.py
pandas/pandas/tests/scalar/interval/test_interval.py
pandas/pandas/tests/scalar/period/test_asfreq.py
pandas/pandas/tests/scalar/period/test_period.py
pandas/pandas/tests/scalar/timedelta/test_arithmetic.py
pandas/pandas/tests/scalar/timedelta/test_construction.py
pandas/pandas/tests/scalar/timedelta/test_timedelta.py
pandas/pandas/tests/scalar/timestamp/test_arithmetic.py
pandas/pandas/tests/scalar/timestamp/test_comparisons.py
pandas/pandas/tests/scalar/timestamp/test_timestamp.py
pandas/pandas/tests/scalar/timestamp/test_timezones.py
pandas/pandas/tests/scalar/timestamp/test_unary_ops.py
pandas/pandas/tests/series/indexing/test_alter_index.py
pandas/pandas/tests/series/test_alter_axes.py
pandas/pandas/tests/series/test_apply.py
pandas/pandas/tests/series/test_arithmetic.py
pandas/pandas/tests/series/test_asof.py
pandas/pandas/tests/series/test_constructors.py
pandas/pandas/tests/series/test_operators.py
pandas/pandas/tests/series/test_timezones.py
pandas/pandas/tests/test_base.py
pandas/pandas/tests/test_common.py
pandas/pandas/tests/test_errors.py
pandas/pandas/tests/test_lib.py
pandas/pandas/tests/test_take.py
pandas/pandas/tests/tseries/offsets/test_offsets.py
pandas/pandas/tests/tseries/offsets/test_ticks.py
pandas/pandas/tests/window/test_dtypes.py
pandas/pandas/tests/window/test_ewm.py
pandas/pandas/tests/window/test_expanding.py
pandas/pandas/tests/window/test_moments.py
pandas/pandas/tests/window/test_rolling.py
pandas/pandas/tests/window/test_timeseries_window.py
pandas/pandas/tests/window/test_window.py

jbrockmendel · 2019-10-16T23:49:49Z

Where in the code_checks script do we have a check for this?

code_checks.sh line 136-138

gfyoung · 2019-10-17T02:22:45Z

That’s checking for a different usage of pytest.raises, which is in the non-context form e.g.

pytest.raises(Exception, lambda f: 1 / 0)

We discourage this form and prefer:

with pytest.raises(Exception):
   1 / 0

I know because I authored the PR that added the check you referenced 😉

simonjayhawkins · 2020-01-14T12:24:50Z

superseded by #30999

gfyoung added Testing pandas testing functions or related to the test suite Code Style Code style, linting, code_checks Needs Discussion Requires discussion from core team before further action labels Nov 26, 2018

gfyoung mentioned this issue Nov 26, 2018

DEPR: Deprecate passing range-like arguments to DatetimeIndex, TimedeltaIndex #23919

Merged

4 tasks

gfyoung added good first issue and removed Needs Discussion Requires discussion from core team before further action labels Nov 26, 2018

gfyoung added a commit to forking-repos/pandas that referenced this issue Dec 1, 2018

TST: Check error message in overlapping bins test

99e7adc

Follow-up to pandas-devgh-23999. xref pandas-devgh-23922.

gfyoung mentioned this issue Dec 1, 2018

TST: Check error message in overlapping bins test #24029

Merged

gfyoung added a commit that referenced this issue Dec 1, 2018

TST: Check error message in overlapping bins test (#24029)

5b6b346

Follow-up to gh-23999. xref gh-23922.

gfyoung mentioned this issue Dec 22, 2018

TST: Add missing match parameter in test_move #24393

Merged

gfyoung added a commit that referenced this issue Dec 22, 2018

TST: Add missing match parameter in test_move (#24393)

aa3d56e

xref gh-23922

simonjayhawkins mentioned this issue Apr 6, 2019

bug in to_html() with formatters=<list> and max_cols #25955

Closed

jbrockmendel closed this as completed Jun 11, 2019

jbrockmendel reopened this Jun 11, 2019

simonjayhawkins mentioned this issue Jun 17, 2019

BUG:Sanity check on merge parameters for correct exception #26824 #26855

Closed

4 tasks

simonjayhawkins mentioned this issue Jul 12, 2019

TST/CLN: Add message checks to raises KeyError tests #27354

Merged

simonjayhawkins added this to the Contributions Welcome milestone Dec 11, 2019

This was referenced Dec 12, 2019

TST: Add message checks to tests/arrays/categorical/ #30242

Merged

TST: Add message checks to tests/arrays/sparse/ #30244

Merged

CLN: more consistent error message for ExtensionDtype.construct_from_string #30247

Merged

ShaharNaveh mentioned this issue Jan 6, 2020

CI: Disallow bare pytest raise #30737

Closed

5 tasks

This was referenced Jan 14, 2020

TST: Insert 'match' to bare pytest raises #30997

Merged

TST: insert 'match' to bare pytest raises in pandas/tests/internals/ #30998

Merged

[Good first issue] TST: Disallow bare pytest.raises #30999

Closed

simonjayhawkins closed this as completed Jan 14, 2020

Aloqeely mentioned this issue Apr 17, 2024

TST: Disallow bare tm.assert_produces_warning #58290

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TST: Disallow bare pytest.raises #23922

TST: Disallow bare pytest.raises #23922

gfyoung commented Nov 26, 2018 •

edited

Loading

datapythonista commented Nov 26, 2018 •

edited

Loading

gfyoung commented Nov 26, 2018

jorisvandenbossche commented Nov 26, 2018

gfyoung commented Nov 26, 2018 •

edited

Loading

gfyoung commented Nov 26, 2018 •

edited

Loading

gfyoung commented Nov 26, 2018 •

edited

Loading

gfyoung commented Nov 26, 2018

WillAyd commented Nov 26, 2018

gfyoung commented Nov 26, 2018 •

edited

Loading

jbrockmendel commented Nov 27, 2018

datapythonista commented Nov 27, 2018

bashtage commented Dec 4, 2018

jbrockmendel commented Dec 18, 2018

gfyoung commented Dec 18, 2018

jbrockmendel commented Dec 18, 2018

jbrockmendel commented Dec 18, 2018

mike-cramblett commented Jan 23, 2019

gfyoung commented Jan 23, 2019 •

edited

Loading

mike-cramblett commented Jan 23, 2019

jbrockmendel commented Jun 11, 2019

jbrockmendel commented Jun 11, 2019

simonjayhawkins commented Jun 11, 2019

WillAyd commented Jun 11, 2019

simonjayhawkins commented Jun 11, 2019

WillAyd commented Jun 11, 2019

simonjayhawkins commented Jun 11, 2019

gfyoung commented Jun 11, 2019

jreback commented Jun 12, 2019

simonjayhawkins commented Jun 12, 2019

jbrockmendel commented Oct 16, 2019

gfyoung commented Oct 16, 2019 •

edited

Loading

jbrockmendel commented Oct 16, 2019

gfyoung commented Oct 17, 2019

simonjayhawkins commented Jan 14, 2020

TST: Disallow bare pytest.raises #23922

TST: Disallow bare pytest.raises #23922

Comments

gfyoung commented Nov 26, 2018 • edited Loading

datapythonista commented Nov 26, 2018 • edited Loading

gfyoung commented Nov 26, 2018

jorisvandenbossche commented Nov 26, 2018

gfyoung commented Nov 26, 2018 • edited Loading

gfyoung commented Nov 26, 2018 • edited Loading

gfyoung commented Nov 26, 2018 • edited Loading

gfyoung commented Nov 26, 2018

WillAyd commented Nov 26, 2018

gfyoung commented Nov 26, 2018 • edited Loading

jbrockmendel commented Nov 27, 2018

datapythonista commented Nov 27, 2018

bashtage commented Dec 4, 2018

jbrockmendel commented Dec 18, 2018

gfyoung commented Dec 18, 2018

jbrockmendel commented Dec 18, 2018

jbrockmendel commented Dec 18, 2018

mike-cramblett commented Jan 23, 2019

gfyoung commented Jan 23, 2019 • edited Loading

mike-cramblett commented Jan 23, 2019

jbrockmendel commented Jun 11, 2019

jbrockmendel commented Jun 11, 2019

simonjayhawkins commented Jun 11, 2019

WillAyd commented Jun 11, 2019

simonjayhawkins commented Jun 11, 2019

WillAyd commented Jun 11, 2019

simonjayhawkins commented Jun 11, 2019

gfyoung commented Jun 11, 2019

jreback commented Jun 12, 2019

simonjayhawkins commented Jun 12, 2019

jbrockmendel commented Oct 16, 2019

gfyoung commented Oct 16, 2019 • edited Loading

jbrockmendel commented Oct 16, 2019

gfyoung commented Oct 17, 2019

simonjayhawkins commented Jan 14, 2020

gfyoung commented Nov 26, 2018 •

edited

Loading

datapythonista commented Nov 26, 2018 •

edited

Loading

gfyoung commented Nov 26, 2018 •

edited

Loading

gfyoung commented Nov 26, 2018 •

edited

Loading

gfyoung commented Nov 26, 2018 •

edited

Loading

gfyoung commented Nov 26, 2018 •

edited

Loading

gfyoung commented Jan 23, 2019 •

edited

Loading

gfyoung commented Oct 16, 2019 •

edited

Loading