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

fix: scalar timestamp assignment (#19843) #19973

Merged
merged 7 commits into from
Aug 2, 2018

Conversation

DylanDmitri
Copy link
Contributor

@DylanDmitri DylanDmitri commented Mar 2, 2018

@gfyoung gfyoung added Datetime Datetime data dtype Dtype Conversions Unexpected or buggy dtype conversions Timedelta Timedelta data type Timezones Timezone data dtype and removed Timedelta Timedelta data type labels Mar 2, 2018
from pandas.core.dtypes.dtypes import DatetimeTZDtype


# referencing #19843: scalar assignment of a tz-aware is object dtype
Copy link
Member

Choose a reason for hiding this comment

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

I think we can find a home for this in another test file (and don't need a new one). In addition, the comment should be under the class declaration.

@jreback : I think tests/frame/test_indexing.py makes sense here. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, should they go under TestDataFrameIndexingDatetimeWithTZ, or in their own class?

Copy link
Member

Choose a reason for hiding this comment

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

Same class sounds good for now.

@@ -3042,6 +3044,22 @@ def test_transpose(self):
expected.index = ['A', 'B']
assert_frame_equal(result, expected)

def test_scalar_assignment(self):
Copy link
Member

Choose a reason for hiding this comment

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

Reference issue number for both tests.

@DylanDmitri
Copy link
Contributor Author

what should I do for the whatsnew?

@gfyoung
Copy link
Member

gfyoung commented Mar 3, 2018

Perhaps something like:

Fixed `DataFrame` assignment with a timezone-aware object (:issue:`19843`)

@codecov
Copy link

codecov bot commented Mar 3, 2018

Codecov Report

Merging #19973 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #19973      +/-   ##
==========================================
+ Coverage   92.06%   92.06%   +<.01%     
==========================================
  Files         169      169              
  Lines       50681    50690       +9     
==========================================
+ Hits        46659    46668       +9     
  Misses       4022     4022
Flag Coverage Δ
#multiple 90.47% <100%> (ø) ⬆️
#single 42.32% <100%> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/frame.py 97.26% <100%> (ø) ⬆️
pandas/io/parsers.py 95.48% <0%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cf14627...1b9649c. Read the comment docs.

@@ -854,6 +854,7 @@ Datetimelike
- Bug in :func:`to_datetime` where passing an out-of-bounds datetime with ``errors='coerce'`` and ``utc=True`` would raise ``OutOfBoundsDatetime`` instead of parsing to ``NaT`` (:issue:`19612`)
- Bug in :class:`DatetimeIndex` and :class:`TimedeltaIndex` addition and subtraction where name of the returned object was not always set consistently. (:issue:`19744`)
- Bug in :class:`DatetimeIndex` and :class:`TimedeltaIndex` addition and subtraction where operations with numpy arrays raised ``TypeError`` (:issue:`19847`)
- Bug in `DataFrame` assignment with a timezone-aware object (:issue:`19843`)
Copy link
Contributor

Choose a reason for hiding this comment

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

use double-backticks are DataFrame

say a timezone-aware scalar

@@ -2868,9 +2868,15 @@ def reindexer(value):
value = maybe_infer_to_datetimelike(value)

else:
# upcast the scalar
# cast ignores pandas dtypes. so save the dtype first
from pandas.core.dtypes.cast import infer_dtype_from_scalar
Copy link
Contributor

Choose a reason for hiding this comment

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

import at the top of the file with other imports

# upcast the scalar
# cast ignores pandas dtypes. so save the dtype first
from pandas.core.dtypes.cast import infer_dtype_from_scalar
pd_dtype, _ = infer_dtype_from_scalar(value, pandas_dtype=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

call the return dtype

value = cast_scalar_to_array(len(self.index), value)
value = maybe_cast_to_datetime(value, value.dtype)

Copy link
Contributor

Choose a reason for hiding this comment

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

you don't need the blank line and the comment here

# issue #19843
df = pd.DataFrame(index=(0, 1, 2))
df['now'] = pd.Timestamp('20130101', tz='UTC')
assert isinstance(df.dtypes[0], DatetimeTZDtype)
Copy link
Contributor

Choose a reason for hiding this comment

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

construct the expected frame and use assert_frame_equal

Copy link
Contributor

Choose a reason for hiding this comment

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

put this in class TestDataFrameIndexingDatetimeWithTZ(TestData)

name it test_setitem_scalar

parameterize it with both a tz and a tz-aware value.

df['now'] = pd.Timestamp('20130101', tz='UTC')
assert isinstance(df.dtypes[0], DatetimeTZDtype)

def test_datetime_index_assignment(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a duplicate test.

Copy link
Contributor

Choose a reason for hiding this comment

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

you could put it as an additional test in the class as above if you'd like (parametrize as well)

@jreback
Copy link
Contributor

jreback commented Apr 14, 2018

can you rebase and update

@jreback
Copy link
Contributor

jreback commented Jul 28, 2018

can you rebase & move the note to 0.24.0

@jreback jreback modified the milestones: 0.24.0, 0.23.4 Aug 2, 2018
@jreback
Copy link
Contributor

jreback commented Aug 2, 2018

fixed up, but going to do in 0.23.4. 0.24.0

@jreback jreback modified the milestones: 0.23.4, 0.24.0 Aug 2, 2018
@jreback jreback merged commit 6e1e1e4 into pandas-dev:master Aug 2, 2018
dberenbaum pushed a commit to dberenbaum/pandas that referenced this pull request Aug 3, 2018
minggli added a commit to minggli/pandas that referenced this pull request Aug 5, 2018
* master: (47 commits)
  Run tests in conda build [ci skip] (pandas-dev#22190)
  TST: Check DatetimeIndex.drop on DST boundary (pandas-dev#22165)
  CI: Fix Travis failures due to lint.sh on pandas/core/strings.py (pandas-dev#22184)
  Documentation: typo fixes in MultiIndex / Advanced Indexing (pandas-dev#22179)
  DOC: added .join to 'see also' in Series.str.cat (pandas-dev#22175)
  DOC: updated Series.str.contains see also section (pandas-dev#22176)
  0.23.4 whatsnew (pandas-dev#22177)
  fix: scalar timestamp assignment (pandas-dev#19843) (pandas-dev#19973)
  BUG: Fix get dummies unicode error (pandas-dev#22131)
  Fixed py36-only syntax [ci skip] (pandas-dev#22167)
  DEPR: pd.read_table (pandas-dev#21954)
  DEPR: Removing previously deprecated datetools module (pandas-dev#6581) (pandas-dev#19119)
  BUG: Matplotlib scatter datetime (pandas-dev#22039)
  CLN: Use public method to capture UTC offsets (pandas-dev#22164)
  implement tslibs/src to make tslibs self-contained (pandas-dev#22152)
  Fix categorical from codes nan 21767 (pandas-dev#21775)
  BUG: Better handling of invalid na_option argument for groupby.rank(pandas-dev#22124) (pandas-dev#22125)
  use memoryviews instead of ndarrays (pandas-dev#22147)
  Remove depr. warning in SeriesGroupBy.count (pandas-dev#22155)
  API: Default to_* methods to compression='infer' (pandas-dev#22011)
  ...
Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request Oct 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Datetime Datetime data dtype Dtype Conversions Unexpected or buggy dtype conversions Timezones Timezone data dtype
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: scalar assignment of a tz-aware is object dtype
3 participants