Skip to content

Conversation

@jbrockmendel
Copy link
Member

cc @jreback @TomAugspurger

couple of issues with fillna needed sorting out

  • The DTA version was operating in-place (fixed+tested)
  • The TDA version would raise because it wasn't supported in core.missing (fixed+tested)
  • The DTA[tz] version would incorrectly re-localize using the existing constructors, i.e. was dependent on the constructor changes in 24024. With the edits here it is correct regardless of whether the constructor is changed.

@jbrockmendel
Copy link
Member Author

kind of weird that Series[timedelta64].fillna(method='pad') works OK in master, will need to track down why it hadn't been supported directly in core.missing.pad_1d

@codecov
Copy link

codecov bot commented Jan 1, 2019

Codecov Report

Merging #24536 into master will decrease coverage by <.01%.
The diff coverage is 12.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #24536      +/-   ##
==========================================
- Coverage   31.88%   31.87%   -0.01%     
==========================================
  Files         166      166              
  Lines       52427    52438      +11     
==========================================
  Hits        16714    16714              
- Misses      35713    35724      +11
Flag Coverage Δ
#multiple 30.27% <12.5%> (-0.01%) ⬇️
#single 31.87% <12.5%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/core/arrays/period.py 41.87% <ø> (+1.26%) ⬆️
pandas/core/missing.py 10.25% <0%> (-0.11%) ⬇️
pandas/core/arrays/datetimelike.py 37.11% <14.28%> (-1.25%) ⬇️

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 091cfbb...aa9f457. Read the comment docs.

@codecov
Copy link

codecov bot commented Jan 1, 2019

Codecov Report

Merging #24536 into master will increase coverage by 60.44%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #24536       +/-   ##
===========================================
+ Coverage   31.88%   92.32%   +60.44%     
===========================================
  Files         166      166               
  Lines       52427    52440       +13     
===========================================
+ Hits        16714    48417    +31703     
+ Misses      35713     4023    -31690
Flag Coverage Δ
#multiple 90.75% <100%> (+60.46%) ⬆️
#single 43% <12.5%> (+11.12%) ⬆️
Impacted Files Coverage Δ
pandas/core/arrays/period.py 98.52% <ø> (+57.91%) ⬆️
pandas/core/arrays/datetimelike.py 96.61% <100%> (+58.24%) ⬆️
pandas/core/missing.py 92.3% <100%> (+81.94%) ⬆️
pandas/util/_test_decorators.py 90.54% <0%> (+4.05%) ⬆️
pandas/compat/__init__.py 57.91% <0%> (+9.65%) ⬆️
pandas/core/config_init.py 99.24% <0%> (+9.84%) ⬆️
pandas/core/reshape/util.py 100% <0%> (+11.53%) ⬆️
pandas/core/api.py 100% <0%> (+13.33%) ⬆️
pandas/compat/numpy/__init__.py 92.85% <0%> (+14.28%) ⬆️
pandas/io/formats/console.py 74.24% <0%> (+16.66%) ⬆️
... and 128 more

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 091cfbb...52e01ae. Read the comment docs.

@jreback jreback added Refactor Internal refactoring of code Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate labels Jan 1, 2019
elif values.dtype == np.object_:
_method = algos.pad_inplace_object
elif is_timedelta64_dtype(values):
# NaTs are treated identically to datetime64, so we can dispatch
Copy link
Contributor

Choose a reason for hiding this comment

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

is something failing? i agree this is probably the soln. but likely these are converted prior to calling this (to i8)

Copy link
Member Author

Choose a reason for hiding this comment

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

Presumably; I haven’t looked at the series code to see how it handles this. Enough core.missing mysteries have popped up today I’ll be giving them a close look after the RC

Copy link
Contributor

Choose a reason for hiding this comment

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

sure

@jbrockmendel
Copy link
Member Author

gentle ping

@jreback jreback added this to the 0.24.0 milestone Jan 1, 2019
@jreback jreback merged commit cf92230 into pandas-dev:master Jan 1, 2019
@jreback
Copy link
Contributor

jreback commented Jan 1, 2019

thanks

@jbrockmendel jbrockmendel deleted the fillna branch January 1, 2019 23:14
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate Refactor Internal refactoring of code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants