Skip to content

TST: insert 'match' to bare pytest raises in pandas/tests/tseries/off… #36865

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 11, 2020
Merged

TST: insert 'match' to bare pytest raises in pandas/tests/tseries/off… #36865

merged 1 commit into from
Oct 11, 2020

Conversation

krajatcl
Copy link
Contributor

@krajatcl krajatcl commented Oct 4, 2020

…sets/test_offsets.py

jbrockmendel
jbrockmendel reviewed Oct 4, 2020
View reviewed changes
pandas/tests/tseries/offsets/test_offsets.py Outdated
@@ -4315,7 +4326,8 @@ def test_valid_month_attributes(kwd, month_classes):
# GH#18226
cls = month_classes
# check that we cannot create e.g. MonthEnd(weeks=3)
with pytest.raises(TypeError):
msg = f"__init__\\(\\) got an unexpected keyword argument '{kwd}'"
Copy link
Member

Choose a reason for hiding this comment

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

can you make these single-slash and use "rf" prefix

@jbrockmendel
Copy link
Member

nitpick, otherwise LGTM

jbrockmendel
jbrockmendel reviewed Oct 7, 2020
View reviewed changes
pandas/tests/tseries/offsets/test_offsets.py Outdated
@@ -4338,32 +4350,34 @@ def test_valid_tick_attributes(kwd, tick_classes):
# GH#18226
cls = tick_classes
# check that we cannot create e.g. Hour(weeks=3)
with pytest.raises(TypeError):
msg = f"__init__\\(\\) got an unexpected keyword argument '{kwd}'"
Copy link
Member

Choose a reason for hiding this comment

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

same here

@jreback jreback added Error Reporting Incorrect or improved errors from pandas Testing pandas testing functions or related to the test suite labels Oct 7, 2020
@jreback jreback added this to the 1.2 milestone Oct 7, 2020
@krajatcl krajatcl requested a review from jbrockmendel October 10, 2020 01:38
@krajatcl
Copy link
Contributor Author

Hi can you add this label
hacktoberfest-accepted

@jreback
Copy link
Contributor

jreback commented Oct 10, 2020

hmm can you merge master and ping on green.

@krajatcl
Copy link
Contributor Author

I did git reset with upstream/master and than ran pytest pandas/tests/tseries/offsets/test_offsets.py -s. And it showed lots of Exceptions raised (293 failed) should i ignore them for now or need to fix them too along with match in pytest.raises.
@jreback

@jreback
Copy link
Contributor

jreback commented Oct 10, 2020

I did git reset with upstream/master and than ran pytest pandas/tests/tseries/offsets/test_offsets.py -s. And it showed lots of Exceptions raised (293 failed) should i ignore them for now or need to fix them too along with match in pytest.raises.
@jreback

really? you should have none

@krajatcl
Copy link
Contributor Author

krajatcl commented Oct 10, 2020 •
edited
Loading

Yes, There are and i have not even made any changes in upstream/master at that point. I am not sure whether it has something to do with my local setup or not. But if i remember correctly at start of this pull request i saw none. I will check with my local setup once again and see if i made any mistake there but i am using same docker setup.

@jreback

@jreback
Copy link
Contributor

jreback commented Oct 10, 2020

Yes, There are and i have not even made any changes in upstream/master at that point. I am not sure whether it has something to do with my local setup or not. But if i remember correctly at start of this pull request i saw none. I will check with my local setup once again and see if i made any mistake there but i am using same docker setup.

@jreback

ok pls just merge master and ping on green

@jreback
Copy link
Contributor

jreback commented Oct 10, 2020

yeah this is actually failing, so please see if you can fixup things.

@krajatcl krajatcl force-pushed the feature branch 4 times, most recently from de1d17e to 84496e6 Compare October 11, 2020 11:28
@krajatcl
Copy link
Contributor Author

hi @jreback
I am new to open source so earlier i only used to do pytest {corresponding file name which i changed} for some of my pull request raised. But as i noticed in this request that some of test cases were failing due to one of my previous PR #37027. On that i did same basic testing which passed but caused failure in testing env. I have fixed those issues in this comment. So can you guide me (or any documentation link) what else i need to do to make sure that my changes don't cause problem later on.
Other command i found in the testing env was
pytest -m "not slow and not network and not clipboard" -n auto --dist=loadfile -s --strict --durations=30 --junitxml=test-data.xml pandas.
Is this enough for initial testing.
Sorry to bother you with this request but any help will be much appreciated.

@jreback
Copy link
Contributor

jreback commented Oct 11, 2020

i think master is now updated, so need to merge master and see if there is anything left to change.

@krajatcl
Copy link
Contributor Author

i think master is now updated, so need to merge master and see if there is anything left to change

Done with rebasing and most of test cases are done only one remaining.

@jreback jreback merged commit 6c1c5c6 into pandas-dev:master Oct 11, 2020
@jreback
Copy link
Contributor

jreback commented Oct 11, 2020

thanks @krajatcl

JulianWgs pushed a commit to JulianWgs/pandas that referenced this pull request Oct 26, 2020
@krajatcl @JulianWgs
TST: insert 'match' to bare pytest raises in pandas/tests/tseries/off…
89551d3 
…sets/test_offsets.py (pandas-dev#36865)

Co-authored-by: Rajat Bishnoi <[email protected]>
kesmit13 pushed a commit to kesmit13/pandas that referenced this pull request Nov 2, 2020
@krajatcl
TST: insert 'match' to bare pytest raises in pandas/tests/tseries/off…
f9bb6aa 
…sets/test_offsets.py (pandas-dev#36865)

Co-authored-by: Rajat Bishnoi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbrockmendel jbrockmendel Awaiting requested review from jbrockmendel

Assignees
No one assigned
Labels
Error Reporting Incorrect or improved errors from pandas Testing pandas testing functions or related to the test suite
Projects
None yet
Milestone
1.2
Development

Successfully merging this pull request may close these issues.
3 participants
@krajatcl @jbrockmendel @jreback