-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
TST: Series construction from ExtensionDtype scalar #37989
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
Conversation
this looks fine. should there be an analogous test in tests.extension? |
Hmmm do you mean something like this? In [7]: pd.array(pd.Interval(0, 1))
(...)
ValueError: Cannot pass scalar '(0, 1]' to 'pandas.array'. We test this here: pandas/pandas/tests/arrays/test_array.py Lines 287 to 290 in 52a1725
and pandas/pandas/tests/arrays/floating/test_construction.py Lines 85 to 108 in 52a1725
|
@@ -95,6 +95,14 @@ def test_scalar_conversion(self): | |||
assert float(Series([1.0])) == 1.0 | |||
assert int(Series([1.0])) == 1 | |||
|
|||
@pytest.mark.parametrize("scalar", (Interval(0, 1), Period("2019Q1", freq="Q"))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you create a fixture of the ea_scalars and use it here: https://github.com/pandas-dev/pandas/blob/master/pandas/tests/frame/test_constructors.py#L738 and in the new test you are adding
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are also similar tests for a dict of a scalar in both series/frame which could be updated to use the fixture as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you create a fixture of the ea_scalars and use it here: https://github.com/pandas-dev/pandas/blob/master/pandas/tests/frame/test_constructors.py#L738 and in the new test you are adding
Done
@@ -95,6 +95,14 @@ def test_scalar_conversion(self): | |||
assert float(Series([1.0])) == 1.0 | |||
assert int(Series([1.0])) == 1 | |||
|
|||
@pytest.mark.parametrize("scalar", (Interval(0, 1), Period("2019Q1", freq="Q"))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are also similar tests for a dict of a scalar in both series/frame which could be updated to use the fixture as well
Timestamp("2011-01-01", tz="US/Eastern"), | ||
DatetimeTZDtype(tz="US/Eastern"), | ||
), | ||
(Timedelta(seconds=500), "timedelta64[ns]"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
xref #38032 for a weird bug when the timedelta is in ns
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great thanks. testing catching edge cases, who would think!
Timestamp("2011-01-01", tz="US/Eastern"), | ||
DatetimeTZDtype(tz="US/Eastern"), | ||
), | ||
(Timedelta(seconds=500), "timedelta64[ns]"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great thanks. testing catching edge cases, who would think!
thanks @arw2019 FYI there maybe other places where we can use this fixture, if its simple, would take a PR, if not, pls open an issue. |
Working on it. Probably will be a single PR but will see |
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff