-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: fix replace_list #27720
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
BUG: fix replace_list #27720
Conversation
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.
Cool thanks. Not super familiar with this but change looks reasonable.
Need a whatsnew?
for a in self._AXIS_ORDERS: | ||
if not len(self._get_axis(a)): | ||
return self | ||
if not self.size: |
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.
Do we even need this check any more or would this happen implicitly?
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'm not sure, I was mostly happy about avoiding the use of AXIS_ORDERS.
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.
general question. is NDFrame supposed to represent a n-dimensional data structure or is it a base class for Series and DataFrame?
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.
NDFrame is just a base class (of Series & DataFrame)
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.
it was also backing Panel / Panel4D, but no longer
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'm thinking should the generic nature of NDFrame be maintained to facilitate a third party Panel implementation, or is that ruled out-of-scope for pandas?
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'm thinking should the generic nature of NDFrame be maintained to facilitate a third party Panel implementation, or is that ruled out-of-scope for pandas?
no that's out of scope. the reason we removed Panel is because of all of the complexitiy related to > 2ndim. xarray is a better platform for that.
generic is just the collection of common api between Series/DataFrame
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.
mypy complained about FrameOrSeries, was OK with NDFrame
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.
in pandas._typing FrameOrSeries is a typevar of Series and DataFrame. This is not applicable to core.generic.
in #27646 FrameOrSeries is defined as a typevar bound by NDFrame so that a series returns a series, a DataFrame returns a DataFrame, a subclassed DataFrame returns a subclassed DataFrame etc.
NDFrame is a nominal type, so allows any subclass of NDFrame to be returned.
added whatsnew |
@@ -152,7 +152,7 @@ ExtensionArray | |||
|
|||
Other | |||
^^^^^ | |||
|
|||
- Bug in :meth:`Series.replace` and :meth:`DataFrame.replace` when replacing timezone-aware timestamps using a dict-like replacer (:issue:`27720`) |
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.
does this close any issues?
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.
Not AFAICT. Searching the tracker doesn't show anything obvious and the xfail comment points to a closed PR.
lgtm. question. |
thanks @jbrockmendel |
@pytest.mark.parametrize( | ||
"from_key", ["datetime64[ns, UTC]", "datetime64[ns, US/Eastern]"] | ||
) | ||
def test_replace_series_datetime_tz(self, how, to_key, from_key): | ||
how = "series" | ||
from_key = "datetime64[ns, US/Eastern]" | ||
to_key = "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.
should these three assignments not be removed now?
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff