Skip to content
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

DEPR: Deprecate use of un-supported numpy dt64/td64 dtype for pandas.array #53817

Merged
merged 10 commits into from Jul 11, 2023

Conversation

rmhowe425
Copy link
Contributor

@rmhowe425 rmhowe425 commented Jun 23, 2023

@rmhowe425 rmhowe425 changed the title Deprecate use of un-supported numpy dt64/td64 dtype for pandas.array DEPR: Deprecate use of un-supported numpy dt64/td64 dtype for pandas.array Jun 23, 2023
@mroeschke mroeschke added Deprecate Functionality to remove in pandas Non-Nano datetime64/timedelta64 with non-nanosecond resolution labels Jun 23, 2023
@rmhowe425
Copy link
Contributor Author

@jbrockmendel PR is ready for inspection

warnings.warn(
r"dt/td64 dtypes with 'm' and 'h' resolutions are deprecated."
r"Supported resolutions are 's', 'ms','us', and 'ns'. "
r"In future releases, 'm' and 'h' resolutions will be cast to the closest "
Copy link
Member

Choose a reason for hiding this comment

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

dont specify "m" and "h", since that leaves out others and it isnt worth listing all of them

@@ -379,6 +381,16 @@ def array(
):
return TimedeltaArray._from_sequence(data, dtype=dtype, copy=copy)

elif lib.is_np_dtype(dtype, "mM"):
warnings.warn(
r"dt/td64 dtypes with 'm' and 'h' resolutions are deprecated."
Copy link
Member

Choose a reason for hiding this comment

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

pls write out datetime64 and timedelta64

@rmhowe425
Copy link
Contributor Author

@jbrockmendel PR has been updated. Pinging on green

1 similar comment
@rmhowe425
Copy link
Contributor Author

@jbrockmendel PR has been updated. Pinging on green

@@ -379,6 +381,16 @@ def array(
):
return TimedeltaArray._from_sequence(data, dtype=dtype, copy=copy)

elif lib.is_np_dtype(dtype, "mM"):
warnings.warn(
r"Deprecating unsupported datetime64 and timedelta64 dtype resolutions. "
Copy link
Member

Choose a reason for hiding this comment

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

the first sentence here is awkward. Maybe "datetime64 and timedelta64 dtypes with resolutions other than ... are deprecated."

If they pass an unsupported dtype, we should raise, not silently cast

@rmhowe425
Copy link
Contributor Author

@jbrockmendel PR has been updated. Pinging on green

r"datetime64 and timedelta64 dtype resolutions other than "
r"'s', 'ms','us', and 'ns' are deprecated. "
r"In future releases passing unsupported resolutions will "
r"raise an exception. ",
Copy link
Member

Choose a reason for hiding this comment

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

typo trailing whitespace

elif lib.is_np_dtype(dtype, "mM"):
warnings.warn(
r"datetime64 and timedelta64 dtype resolutions other than "
r"'s', 'ms','us', and 'ns' are deprecated. "
Copy link
Member

Choose a reason for hiding this comment

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

missing space between ms and us

@rmhowe425
Copy link
Contributor Author

@jbrockmendel PR has been updated. Pinging on green

@jbrockmendel
Copy link
Member

@mroeschke worth nitpicking wording any further?

@@ -28,6 +29,22 @@
)


def test_dt64_array():
# PR 53817
dtype_unit_lst = ["M8[h]", "M8[m]", "m8[h]", "M8[m]"]
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Could you use @pytest.mark.parameterize here?

@rmhowe425 rmhowe425 requested a review from mroeschke July 11, 2023 00:59
@rmhowe425
Copy link
Contributor Author

@mroeschke PR ready for inspection. Looks like unit test test_indices_concatenation_order is causing unit tests to fail. Seems to be a known issue.

@mroeschke mroeschke added this to the 2.1 milestone Jul 11, 2023
@mroeschke mroeschke merged commit b4929f8 into pandas-dev:main Jul 11, 2023
16 of 36 checks passed
@mroeschke
Copy link
Member

Thanks @rmhowe425

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecate Functionality to remove in pandas Non-Nano datetime64/timedelta64 with non-nanosecond resolution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API: pd.array convert unsupported dt64/td64 to supported?
3 participants