Skip to content

TST: setitem preserving period[D] dtype #52704

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

Conversation

srkds
Copy link
Contributor

@srkds srkds commented Apr 17, 2023

expected = df.dtypes
df.iloc[:] = rng._na_value
result = df.dtypes
tm.assert_equal(result, expected)
Copy link
Member

Choose a reason for hiding this comment

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

Please construct the expected DataFrame and use tm.assert_frame_equal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please construct the expected DataFrame and use tm.assert_frame_equal

I'm a bit confused here, I thought that way but after executing this line df.iloc[:] = rng._na_value we get following o/p

# o/p of  df.iloc[:] = rng._na_value
	A
0	NaT
1	NaT
2	NaT
3	NaT
4	NaT
5	NaT
6	NaT
7	NaT
8	NaT

# And it's data type
# -> df.dtypes
A    period[D]
dtype: object

and that was a result.

but for expected when I tried creating the same data frame and changing its type to period[D] it did not work.

expected = pd.DataFrame({"A": ['NaT', 'NaT', 'NaT', 'NaT', 'NaT', 'NaT', 'NaT', 'NaT', 'NaT']})
expected.astype(obj.dtypes) # 1 did not work
expected.astype("period[D]") # 2 did not work
expected.dtypes

# o/p
A    object
dtype: object

as per your suggestion, we can create a new data frame using period_range and compare using tm.assert_frame_equal here dtypes would be the same but the data is different so the test will fail.

I hope I'm able to describe the confusion clearly here.
Please let me know your thoughts on this.

Copy link
Member

Choose a reason for hiding this comment

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

In [3]: pd.DataFrame({"A": ["NaT"]}, dtype="period[D]")
Out[3]: 
     A
0  NaT

In [4]: pd.DataFrame({"A": ["NaT"]}, dtype="period[D]").dtypes
Out[4]: 
A    period[D]
dtype: object

@mroeschke mroeschke added the Testing pandas testing functions or related to the test suite label Apr 17, 2023
@@ -379,6 +379,15 @@ def test_setitem_complete_column_with_array(self):
assert expected["d"].dtype == arr.dtype
tm.assert_frame_equal(df, expected)

def test_setitem_periodd_dtype(self):
Copy link
Member

Choose a reason for hiding this comment

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

typo extra "d"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

typo extra "d"

Originally its period[D] but to name a function I thought this would be fine.

But I think there are ways we can rename

  1. ..._period_d_dtpye(self)
  2. ..._period_dtype(self)

What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

_period_d_dtype is fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay! sounds good 👍
Thanks

@mroeschke mroeschke added this to the 2.1 milestone Apr 19, 2023
@mroeschke mroeschke merged commit 7ab653d into pandas-dev:main Apr 19, 2023
@mroeschke
Copy link
Member

Thanks @srkds

@srkds
Copy link
Contributor Author

srkds commented Apr 19, 2023

Thanks @mroeschke and @jbrockmendel for review ❤😀

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.

BUG: setitem with ArrayManager not preserving PeriodDtype
3 participants