Skip to content

add match to bare pyteset.raises() - arrays #33010

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 3 commits into from
Mar 30, 2020

Conversation

sathyz
Copy link
Contributor

@sathyz sathyz commented Mar 25, 2020

@ShaharNaveh
Copy link
Member

* [x]  closes #30999

* [x]  tests added / passed

* [x]  passes `black pandas`

* [x]  passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

* [ ]  whatsnew entry

@sathyz Can you please change the word "closes" to the word "ref"?

@@ -57,7 +57,7 @@ def test_overlaps_interval_container(self, constructor, other_constructor):
# TODO: modify this test when implemented
interval_container = constructor.from_breaks(range(5))
other_container = other_constructor.from_breaks(range(5))
with pytest.raises(NotImplementedError):
with pytest.raises(NotImplementedError, match=None):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
with pytest.raises(NotImplementedError, match=None):
with pytest.raises(NotImplementedError, match="^$"):

Comment on lines 798 to 813
msg = (
r"'value' should be a 'Timestamp', 'NaT', or array of those. "
r"Got 'timedelta64' instead.|"
r"'value' should be a 'Timedelta', 'NaT', or array of those. "
r"Got 'datetime64' instead.|"
r"'value' should be a 'Timedelta', 'NaT', or array of those. "
r"Got 'int' instead.|"
r"'value' should be a 'Timestamp', 'NaT', or array of those. "
r"Got 'int' instead.|"
r"'value' should be a 'Period', 'NaT', or array of those. "
r"Got 'datetime64' instead.|"
r"'value' should be a 'Period', 'NaT', or array of those. "
r"Got 'timedelta64' instead.|"
r"'value' should be a 'Period', 'NaT', or array of those. "
r"Got 'int' instead."
)
Copy link
Member

Choose a reason for hiding this comment

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

We are trying to use the pattern of:

msg = "|".join(
    [
        "foo",
        "bar,
        "baz"
    ]
)

Copy link
Member

Choose a reason for hiding this comment

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

After having a second look, I think we can have this as a single string

Suggested change
msg = (
r"'value' should be a 'Timestamp', 'NaT', or array of those. "
r"Got 'timedelta64' instead.|"
r"'value' should be a 'Timedelta', 'NaT', or array of those. "
r"Got 'datetime64' instead.|"
r"'value' should be a 'Timedelta', 'NaT', or array of those. "
r"Got 'int' instead.|"
r"'value' should be a 'Timestamp', 'NaT', or array of those. "
r"Got 'int' instead.|"
r"'value' should be a 'Period', 'NaT', or array of those. "
r"Got 'datetime64' instead.|"
r"'value' should be a 'Period', 'NaT', or array of those. "
r"Got 'timedelta64' instead.|"
r"'value' should be a 'Period', 'NaT', or array of those. "
r"Got 'int' instead."
)
msg = (
"'value' should be a '(Timestamp|Timedelta|Period)', 'NaT', or array of those. "
"Got '(timedelta64|datetime64|int)' instead."
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

assertions fail when I use this,

E               AssertionError: Pattern "'value' should be a '(Timestamp|Timedelta|Period)','NaT', or array of those. Got '(timedelta64|datetime64|int)' instead." not found in "'value' should be a 'Timedelta', 'NaT', or array of those. Got 'datetime64' instead."

I'm trying to investigate as follows,

>>> import re
>>> msg = "'value' should be a 'Timedelta', 'NaT', or array of those. Got 'datetime64' instead."
>>> pat = r"'value' should be a '(Timestamp|Timedelta|Period)', 'NaT', or array of those. Got '(timedelta64|datetimee64|int)' instead."
>>> re.search(pat, msg)
>>>

Copy link
Member

Choose a reason for hiding this comment

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

it looks like missing a space after comma

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah my bad.

@ShaharNaveh ShaharNaveh added the Testing pandas testing functions or related to the test suite label Mar 25, 2020
@@ -108,14 +108,20 @@ def test_take_fill(self):
result = arr.take([-1, 1], allow_fill=True, fill_value=pd.NaT)
assert result[0] is pd.NaT

with pytest.raises(ValueError):
arr.take([0, 1], allow_fill=True, fill_value=2)
value = 2
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 split this out from test_take_fill as a separate test, say test_take_fill_raises and parametrize over value.

@jreback jreback added this to the 1.1 milestone Mar 30, 2020
@jreback jreback merged commit 914c390 into pandas-dev:master Mar 30, 2020
@jreback
Copy link
Contributor

jreback commented Mar 30, 2020

thanks @sathyz

@sathyz sathyz deleted the issue-30999-1 branch March 31, 2020 03:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants