-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: Do not use string Index like Datetimelike Index #33531
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
so the point is that you dont want cc @jorisvandenbossche @TomAugspurger xref #33558 |
I'd want to deprecate that behavior. |
If we are to deprecated this behavior, I have some question on how the depreciation it is expected to behave. |
So to make it explicit, currently we have this behaviour:
So a object dtype index with strings, and a DatetimeIndex. Comparing them elementwise correctly gives False values:
but calling the
And I think we want this to give False. Correct? The same also happens for other "coercible" values (not just strings), at least when DatetimeIndex is the calling index. For example:
I would say we also want to deprecate this behaviour? So some questions around this:
|
we should deprecate this |
From here pandas/pandas/core/indexes/datetimelike.py Line 882 in b630cdb
you can see that many types where already excluded from being coerced.
Maybe instead of an exclusion list, we could have an list of which type can could be coerced.
Union and intersection behavior follow equals behavior. The same is true for join and merge.
|
@nrebena i think the idea of this is fine, can you merge master and can re-evaluate |
d442a9f
to
9fc201d
Compare
@jreback Rebased. pandas/pandas/core/indexes/datetimelike.py Line 882 in b630cdb
string typed index eventually.
|
pandas/core/indexes/datetimelike.py
Outdated
@@ -903,6 +926,15 @@ def _is_convertible_to_index_for_join(cls, other: Index) -> bool: | |||
return True | |||
return False | |||
|
|||
def _wrap_joined_index(self, joined: np.ndarray, other): |
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.
why is this added?
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.
My bad I failed my rebase, this is old code I missed.
@@ -408,3 +408,20 @@ def test_isocalendar_returns_correct_values_close_to_new_year_with_tz(): | |||
dtype="UInt32", | |||
) | |||
tm.assert_frame_equal(result, expected_data_frame) | |||
|
|||
|
|||
def test_datetimelike_string(): |
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.
ideally could paramaterize this with things that don't match here (e.g. more than just 1 thing).
Also this is not in the right location, search for were we test .equals
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.
I will move this in pandas/tests/indexes/datetimes/test_ops.py
By trying to parametrize I have now more question about the expected behaviors, see comment that follows.
By trying to parametrize, I did see more case of index of different type that are equals. import pandas as pd
i1 = pd.Index([1586736000.0])
i2 = pd.Index(pd.to_datetime([1586736000.0]))
print(i1)
print(i2)
print(i1.equals(i2))
# Float64Index([1586736000.0], dtype='float64')
# DatetimeIndex(['1970-01-01 00:00:01.586736'], dtype='datetime64[ns]', freq=None)
# True I am not sure this is not the expected behavior either? |
can u merge master and can look again |
@nrebena can you add a release note |
Closing as stale, @nrebena let us know if you'd like to reopen |
Follow up of conversation in #32739 (comment).
The goal is to prevent index of string that look like datetimelike to be used as datetimelike.
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff