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

BUG: fix replace_list #27720

Merged
merged 3 commits into from Aug 5, 2019

Conversation

@jbrockmendel
Copy link
Member

commented Aug 2, 2019

  • tests added / passed
  • passes black pandas
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry
@WillAyd
Copy link
Member

left a comment

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:

This comment has been minimized.

Copy link
@WillAyd

WillAyd Aug 2, 2019

Member

Do we even need this check any more or would this happen implicitly?

This comment has been minimized.

Copy link
@jbrockmendel

jbrockmendel Aug 2, 2019

Author Member

I'm not sure, I was mostly happy about avoiding the use of AXIS_ORDERS.

This comment has been minimized.

Copy link
@simonjayhawkins

simonjayhawkins Aug 5, 2019

Member

general question. is NDFrame supposed to represent a n-dimensional data structure or is it a base class for Series and DataFrame?

This comment has been minimized.

Copy link
@jreback

jreback Aug 5, 2019

Contributor

NDFrame is just a base class (of Series & DataFrame)

This comment has been minimized.

Copy link
@jreback

jreback Aug 5, 2019

Contributor

it was also backing Panel / Panel4D, but no longer

This comment has been minimized.

Copy link
@simonjayhawkins

simonjayhawkins Aug 5, 2019

Member

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?

This comment has been minimized.

Copy link
@jreback

jreback Aug 5, 2019

Contributor

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

This comment has been minimized.

Copy link
@jbrockmendel

jbrockmendel Aug 5, 2019

Author Member

mypy complained about FrameOrSeries, was OK with NDFrame

This comment has been minimized.

Copy link
@simonjayhawkins

simonjayhawkins Aug 5, 2019

Member

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.

@WillAyd WillAyd added the Timezones label Aug 2, 2019

@jbrockmendel

This comment has been minimized.

Copy link
Member Author

commented Aug 2, 2019

added whatsnew

@gfyoung
gfyoung approved these changes Aug 2, 2019

@jreback jreback added this to the 0.25.1 milestone Aug 4, 2019

@@ -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`)

This comment has been minimized.

Copy link
@jreback

jreback Aug 4, 2019

Contributor

does this close any issues?

This comment has been minimized.

Copy link
@jbrockmendel

jbrockmendel Aug 4, 2019

Author Member

Not AFAICT. Searching the tracker doesn't show anything obvious and the xfail comment points to a closed PR.

@jreback

This comment has been minimized.

Copy link
Contributor

commented Aug 4, 2019

lgtm. question.

@jreback jreback merged commit 7d8eeff into pandas-dev:master Aug 5, 2019

15 checks passed

codecov/patch 100% of diff hit (target 50%)
Details
codecov/project 93% (target 82%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
pandas-dev.pandas Build #20190802.60 succeeded
Details
pandas-dev.pandas (Checks) Checks succeeded
Details
pandas-dev.pandas (Docs) Docs succeeded
Details
pandas-dev.pandas (Linux py35_compat) Linux py35_compat succeeded
Details
pandas-dev.pandas (Linux py36_32bit) Linux py36_32bit succeeded
Details
pandas-dev.pandas (Linux py36_locale_slow) Linux py36_locale_slow succeeded
Details
pandas-dev.pandas (Linux py36_locale_slow_old_np) Linux py36_locale_slow_old_np succeeded
Details
pandas-dev.pandas (Linux py37_locale) Linux py37_locale succeeded
Details
pandas-dev.pandas (Linux py37_np_dev) Linux py37_np_dev succeeded
Details
pandas-dev.pandas (Windows py36_np15) Windows py36_np15 succeeded
Details
pandas-dev.pandas (Windows py37_np141) Windows py37_np141 succeeded
Details
pandas-dev.pandas (macOS py35_macos) macOS py35_macos succeeded
Details
@jreback

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2019

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]"

This comment has been minimized.

Copy link
@simonjayhawkins

simonjayhawkins Aug 5, 2019

Member

should these three assignments not be removed now?

jreback added a commit that referenced this pull request Aug 5, 2019
jbrockmendel added a commit to jbrockmendel/pandas that referenced this pull request Aug 5, 2019

@jbrockmendel jbrockmendel deleted the jbrockmendel:replace branch Aug 5, 2019

simonjayhawkins added a commit that referenced this pull request Aug 5, 2019
quintusdias added a commit to quintusdias/pandas_dev that referenced this pull request Aug 16, 2019
quintusdias added a commit to quintusdias/pandas_dev that referenced this pull request Aug 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.