Skip to content

DOC: added type annotations and tests to check for multiple warning match #47833

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 21 commits into from
Aug 3, 2022
Merged

DOC: added type annotations and tests to check for multiple warning match #47833

merged 21 commits into from
Aug 3, 2022

Conversation

shourya5
Copy link
Contributor

@pep8speaks
Copy link

pep8speaks commented Jul 23, 2022

Hello @shourya5! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-08-03 05:49:49 UTC

@shourya5
Copy link
Contributor Author

@github-actions pre-commit

@mroeschke mroeschke added Testing pandas testing functions or related to the test suite Docs Warnings Warnings that appear or should be added to pandas labels Jul 25, 2022
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left some comments - could you address the linting errors too please?

@MarcoGorelli
Copy link
Member

@github-actions pre-commit

this was removed not long ago (the stable docs will only update after the 1.5 release, but the dev docs already don't show it) - can you run it locally?

Just

pre-commit run --files <file_1> <file_2> ... <file_n>

where you put any files you've modified after --files

@shourya5
Copy link
Contributor Author

shourya5 commented Aug 1, 2022

I am failing one test besides the pre-commit test

@MarcoGorelli
Copy link
Member

You need to update the type annotation in _assert_caught_no_extra_warnings too

@shourya5
Copy link
Contributor Author

shourya5 commented Aug 1, 2022

Does this look correct?

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

Couple of things:

  • the docstring of assert produces warning needs updating, can you add a line there about how to catch multiple warnings you can pass them as a tuple? Line 40, in the description of "expected warning"
  • can you run pre-commit on the files you've modified?

@shourya5
Copy link
Contributor Author

shourya5 commented Aug 2, 2022

the pre-commit action is changing 'Tuple' to tuple. Should I change it or leave it as is?

@MarcoGorelli
Copy link
Member

Yeah that's fine, just git add, git commit, git push

@shourya5
Copy link
Contributor Author

shourya5 commented Aug 2, 2022

I think it should pass all tests now

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks @shourya5 !

@MarcoGorelli MarcoGorelli added this to the 1.5 milestone Aug 2, 2022
@MarcoGorelli MarcoGorelli merged commit 7fac8c8 into pandas-dev:main Aug 3, 2022
noatamir pushed a commit to noatamir/pandas that referenced this pull request Nov 9, 2022
…atch (pandas-dev#47833)

* Testing

* TST: added a test for multiple warnings

* TYP: added type annotation  Tuple[Type[Warning], ...] in assert_produces_warning function

* TST: added a test for multiple warnings

* TYP: Fixed Annotations

* TST: Test for catching multiple warnings

* TST: Test for catching multiple warnings

* TST: Test for catching multiple warnings

* TYP : fixed type annotations in _assert_caught_no_extra_warnings

* TYP : updated type annotations

* DOC : added issue as inline comment

* DOC : added some documentation regarding type annotations

* DOC : fixed pre-commit errors

* fix missing whitespace

Co-authored-by: Marco Edward Gorelli <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs Testing pandas testing functions or related to the test suite Warnings Warnings that appear or should be added to pandas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DOC: clarify that assert_produces_warning can take multiple warnings
4 participants