Skip to content

BUG: Series.ffill() with mixed dtypes containing tz-aware datetimes f… #14960

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

Closed
wants to merge 1 commit into from

Conversation

grutts
Copy link

@grutts grutts commented Dec 22, 2016

…ails. (GH14956)

Seems to work with all the datetime classes usually encountered, although 'tz' seems to be the idiom in the codebase (not sure why?). If both need to be supported I can replace getattr(val, 'tzinfo', None) with getattr(val, 'tz', None) or getattr(val, 'tzinfo', None), thus also giving precedence to the former (if available).

(breaking commit was 4de83d2)

@codecov-io
Copy link

codecov-io commented Dec 22, 2016

Current coverage is 84.65% (diff: 100%)

Merging #14960 into master will decrease coverage by <.01%

@@             master     #14960   diff @@
==========================================
  Files           144        144          
  Lines         51043      51043          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits          43214      43213     -1   
- Misses         7829       7830     +1   
  Partials          0          0          

Powered by Codecov. Last update 45910ae...8550ed1

@@ -300,3 +300,4 @@ Bug Fixes
- Bug in ``Series.unique()`` in which unsigned 64-bit integers were causing overflow (:issue:`14721`)
- Require at least 0.23 version of cython to avoid problems with character encodings (:issue:`14699`)
- Bug in converting object elements of array-like objects to unsigned 64-bit integers (:issue:`4471`)
- Bug in Series.ffill() with mixed dtypes containing tz-aware datetimes. (:issue: `14956`)
Copy link
Contributor

Choose a reason for hiding this comment

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

no space after the (:issue:`14956`)

@jreback
Copy link
Contributor

jreback commented Dec 23, 2016

pls add a test in pandas.types.tests.test_inference for the inference check
and a test in series.tests.test_missing (e.g. the same exact test of the issue you had)

otherwise lgtm.

@jreback jreback added Bug Dtype Conversions Unexpected or buggy dtype conversions Datetime Datetime data dtype labels Dec 23, 2016
@jreback jreback added this to the 0.20.0 milestone Dec 23, 2016
@grutts grutts force-pushed the pandas_issue_14956 branch from 20d96e0 to 4c6b9ba Compare December 23, 2016 11:43
@grutts
Copy link
Author

grutts commented Dec 23, 2016

Added some simple tests and corrected whitespace issue. Please let me know if you'd like some more extensive/different testing.

@jreback
Copy link
Contributor

jreback commented Dec 23, 2016

@grutts lgtm. ping on green.

@grutts grutts force-pushed the pandas_issue_14956 branch from 4c6b9ba to 8550ed1 Compare December 23, 2016 15:10
@grutts
Copy link
Author

grutts commented Dec 23, 2016

@jreback all green. Had to make another commit to pass PEP8

@jreback jreback closed this in 97b4295 Dec 23, 2016
@jreback
Copy link
Contributor

jreback commented Dec 23, 2016

thanks!

note I had to add an option to the test_inference test to make it fail w/o the change. The importance of testing first before the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Datetime Datetime data dtype Dtype Conversions Unexpected or buggy dtype conversions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: Series.ffill() of Series with mixed dtypes containing tz-aware datetimes, fails.
3 participants