pandas icon indicating copy to clipboard operation
pandas copied to clipboard

TST: Added script to enforce usage of match argument for tm.assert_produces_warning

Open chaarvii opened this issue 1 year ago • 5 comments

Added a script called enforce_match_arg_in_assert_produces_warning.py which checks that all usages of tm.assert_produces_warning make use of the match argument. This is done to ensure that users are always displayed the correct warning message. Uses of tm.assert_produces_warning(None) have been excluded as no warning is produced in this case.

Furthermore, to ensure that the existing code doesn't fail the pre-commit stage, all files that currently do not use the match argument have been excluded.

Once the PR is merged in, we can open up another issue to shorten the list of "excluded" files.

chaarvii avatar Jul 02 '24 22:07 chaarvii

@Aloqeely Just fixed the issue with Sequence import. It works now with TYPE_CHECKING. Is there anything else left to be addressed? If not, is it good to merge?

chaarvii avatar Jul 05 '24 21:07 chaarvii

LGTM. cc @mroeschke

Aloqeely avatar Jul 05 '24 23:07 Aloqeely

@mroeschke - are we going ahead with this PR? Any reason we’d like to hold it off for now? cc @Aloqeely

chaarvii avatar Jul 13 '24 05:07 chaarvii

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

github-actions[bot] avatar Aug 26 '24 00:08 github-actions[bot]

Thanks for the contribution, and sorry this was not reviewed before. Maybe silly question, but since assert_produces_warning is our own function, why would we want to have a script to check if it's being called with the match parameter, when we can simply make the parameter required?

I'm ok with adding match (not sure if there are cases where it doesn't make sense). But this seems an overkill.

Maybe worth start by fixing some of the cases in somehow small PRs, and if that looks good, then just raise if match is not called (assuming there is agreement).

What do you think?

datapythonista avatar Jun 06 '25 17:06 datapythonista

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

github-actions[bot] avatar Jul 08 '25 00:07 github-actions[bot]

@chaarvii Thanks for the PR.

Closing to clear queue. we can reopen if you're still interested in working on this.

simonjayhawkins avatar Jul 08 '25 09:07 simonjayhawkins