BUG: Change of behavior in casting of datetime-like types in MultiIndex #44081
Conversation
|
Hello @shubham11941140! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2021-10-28 01:57:38 UTC |
alimcmaster1
left a comment
There was a problem hiding this comment.
Thanks for the PR - some comments above
|
|
||
|
|
||
| def test_set_index_MultiIndex(): | ||
| import datetime as dt |
There was a problem hiding this comment.
imports should be at the top of the file?
| name='date', | ||
| freq=None | ||
| ) | ||
| for i in range(len(ex)): |
There was a problem hiding this comment.
Please use this to assert the df are equal:
There was a problem hiding this comment.
I tried it, the issue is that if you print the expected and result, they are exactly the same but the assert_frame_equal is failing.
There was a problem hiding this comment.
tm.assert_index_equal works as I am comparing indexes and not DataFrames.
pandas/core/indexes/multi.py
Outdated
| """ | ||
| level = self._get_level_number(level) | ||
| values = self._get_level_values(level) | ||
| import pandas as pd |
There was a problem hiding this comment.
We shouldn't need to import pandas here - can just import the to_datetime func?
pandas/core/indexes/multi.py
Outdated
| level = self._get_level_number(level) | ||
| values = self._get_level_values(level) | ||
| import pandas as pd | ||
| try: |
There was a problem hiding this comment.
This implementation doesn't seem right.. What are you trying to do?
There was a problem hiding this comment.
I am trying to ensure the behavior of DatetimeIndex reverts to 1.2.5
There was a problem hiding this comment.
Since this seems to be a regression, please investigate what changed and caused the issue and try to restore the original behavior. Calling try except here should not be necessary
There was a problem hiding this comment.
I have removed the try except block
|
Please see contributing guide for help: https://pandas.pydata.org/pandas-docs/stable/development/contributing.html |
|
|
||
|
|
||
| def test_set_index_MultiIndex(): | ||
| df = DataFrame( |
There was a problem hiding this comment.
since you're testing the DataFrame behavior, this probably goes in the tests/frame/methods file
There was a problem hiding this comment.
Actually in this file, there are a lot of tests regarding setting of indexes, hence I have kept this file.
| @@ -1,3 +1,4 @@ | |||
| import datetime as dt | |||
| from datetime import datetime | |||
There was a problem hiding this comment.
import date here instead of importing 'dt'
jreback
left a comment
There was a problem hiding this comment.
pls look at the 1.3 release. I am pretty sure we change this on purpose. datetime.date is retained which is the point here.
| """ | ||
| name = self._name if name is no_default else name | ||
| u = self._simple_new(values, name=name) | ||
| if all(isinstance(x, date) for x in u): |
There was a problem hiding this comment.
this check absolutely doesn't belong here
|
Thanks @shubham11941140 for the PR. These changes do not appear to correspond with the changes in the PR that was identified in the issue as causing the reported change in behavior. We also need to decide if we are reverting to 1.2.5 behavior. I am closing for now but feel free to contribute to the discussion in #43091 |
Added try-except block in multi.py to reset the behaviour back to 1.2.5