- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 19.2k
PDEP6 implementation pt 1: block.setitem, block.putmask #50626
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
Changes from 102 commits
ccfcb8d
              30dde08
              d2dfa3c
              e4ea519
              e160d7f
              f7971f0
              373a30e
              26ab49d
              cc7cc5a
              0362481
              120aee7
              7972c08
              a16dcb8
              25dc26f
              d45b312
              e4ed811
              ace5e05
              34a6194
              0cfefa4
              150fa9a
              1cc3f50
              3f15670
              d0de3f1
              dc6c60b
              e33563a
              37e0520
              2dd85ab
              24ca7c2
              3aed02f
              25f0693
              0e5fb73
              9f56342
              9b9e975
              2f980c6
              044227f
              741b37d
              c536d8a
              142992e
              4551855
              f6e34ef
              01b0c72
              d9c2225
              6676bad
              df740e6
              9a54956
              8386032
              39decb2
              7549888
              527fa2d
              caa35c3
              cc1542d
              6636a37
              492d443
              eb8dd0f
              90a64ab
              272b735
              b3f6b93
              b8532cc
              81bba3c
              87922b5
              f314dd1
              6b5bc73
              8cad201
              d3b12ab
              adc0022
              d5bdfcf
              37836e5
              1a23fe7
              dfa8ed2
              3efe0a5
              cc95eec
              4ba259d
              c21aa4d
              05ffc27
              6612690
              d1aba37
              adaf4e4
              f194434
              6dc7fd1
              82ecca2
              a9d5891
              0ccb541
              aec0c87
              72e5609
              1455263
              d16eea6
              25198d4
              ba6daa5
              9b15bc2
              ebd1a50
              a835281
              908430b
              9802696
              5ee4ebf
              bb37b09
              f46f7e3
              7df15e9
              f323d2a
              5f5a6a5
              0fb017b
              e540404
              dce5f36
              095048c
              0381c62
              e04a8c1
              a843e30
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -425,13 +425,12 @@ Note that this also changes the sum of an empty ``Series``. Previously this alwa | |
| In [1]: pd.Series([]).sum() | ||
| Out[1]: 0 | ||
|  | ||
| but for consistency with the all-NaN case, this was changed to return NaN as well: | ||
| but for consistency with the all-NaN case, this was changed to return 0 as well: | ||
|  | ||
| .. ipython:: python | ||
| :okwarning: | ||
|  | ||
| pd.Series([]).sum() | ||
| .. code-block:: ipython | ||
|  | ||
| In [2]: pd.Series([]).sum() | ||
| Out[2]: 0 | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is unrelated to this PR, just incorrect AFAICT. (mostly a note-to-self) | ||
|  | ||
| .. _whatsnew_0210.api_breaking.loc: | ||
|  | ||
|  | @@ -755,10 +754,16 @@ Previously assignments, ``.where()`` and ``.fillna()`` with a ``bool`` assignmen | |
|  | ||
| New behavior | ||
|  | ||
| .. ipython:: python | ||
| .. code-block:: ipython | ||
|  | ||
| s[1] = True | ||
| s | ||
| In [7]: s[1] = True | ||
|  | ||
| In [8]: s | ||
| Out[8]: | ||
| 0 1 | ||
| 1 True | ||
| 2 3 | ||
| Length: 3, dtype: object | ||
|  | ||
| Previously, as assignment to a datetimelike with a non-datetimelike would coerce the | ||
| non-datetime-like item being assigned (:issue:`14145`). | ||
|  | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -12,6 +12,7 @@ | |
| cast, | ||
| final, | ||
| ) | ||
| import warnings | ||
|  | ||
| import numpy as np | ||
|  | ||
|  | @@ -42,6 +43,7 @@ | |
| ) | ||
| from pandas.errors import AbstractMethodError | ||
| from pandas.util._decorators import cache_readonly | ||
| from pandas.util._exceptions import find_stack_level | ||
| from pandas.util._validators import validate_bool_kwarg | ||
|  | ||
| from pandas.core.dtypes.astype import ( | ||
|  | @@ -414,7 +416,7 @@ def split_and_operate(self, func, *args, **kwargs) -> list[Block]: | |
| # Up/Down-casting | ||
|  | ||
| @final | ||
| def coerce_to_target_dtype(self, other) -> Block: | ||
| def coerce_to_target_dtype(self, other, warn_on_upcast: bool = False) -> Block: | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this could be changed to  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this would also cover 'shift' (xref #53802). I don't recall that being a part of PDEP6. Am I remembering wrong? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh i see there's a new kwd, never mind | ||
| """ | ||
| coerce the current block to a dtype compat for other | ||
| we will return a block, possibly object, and not raise | ||
|  | @@ -423,7 +425,21 @@ def coerce_to_target_dtype(self, other) -> Block: | |
| and will receive the same block | ||
| """ | ||
| new_dtype = find_result_type(self.values, other) | ||
|  | ||
| if warn_on_upcast: | ||
| warnings.warn( | ||
| f"Setting an item of incompatible dtype is deprecated " | ||
| "and will raise in a future error of pandas. " | ||
| f"Value '{other}' has dtype incompatible with {self.dtype}, " | ||
| "please explicitly cast to a compatible dtype first.", | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thinking aloud. It would be difficult to suggest which dtype to cast? I guess  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in practice i think it will almost always be x->object or int->float There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe - but I'd suggest doing this separately if it's OK, this has already been open since January 😄 reckon we can get this in for 2.1? It should be pretty quick to rebase #53405, can do that right after this one There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep this would be fine as a follow up There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Works for me | ||
| FutureWarning, | ||
| stacklevel=find_stack_level(), | ||
| ) | ||
| if self.dtype == new_dtype: | ||
| raise AssertionError( | ||
| f"Did not expect new dtype {new_dtype} to equal self.dtype " | ||
| f"{self.dtype}. Please report a bug at " | ||
| "https://github.com/pandas-dev/pandas/issues." | ||
| ) | ||
| return self.astype(new_dtype, copy=False) | ||
|  | ||
| @final | ||
|  | @@ -1073,7 +1089,7 @@ def setitem(self, indexer, value, using_cow: bool = False) -> Block: | |
| casted = np_can_hold_element(values.dtype, value) | ||
| except LossySetitemError: | ||
| # current dtype cannot store value, coerce to common dtype | ||
| nb = self.coerce_to_target_dtype(value) | ||
| nb = self.coerce_to_target_dtype(value, warn_on_upcast=True) | ||
|         
                  jbrockmendel marked this conversation as resolved.
              Show resolved
            Hide resolved | ||
| return nb.setitem(indexer, value) | ||
| else: | ||
| if self.dtype == _dtype_obj: | ||
|  | @@ -1146,7 +1162,9 @@ def putmask(self, mask, new, using_cow: bool = False) -> list[Block]: | |
|  | ||
| if not is_list_like(new): | ||
| # using just new[indexer] can't save us the need to cast | ||
| return self.coerce_to_target_dtype(new).putmask(mask, new) | ||
| return self.coerce_to_target_dtype( | ||
| new, warn_on_upcast=True | ||
| ).putmask(mask, new) | ||
| else: | ||
| indexer = mask.nonzero()[0] | ||
| nb = self.setitem(indexer, new[indexer], using_cow=using_cow) | ||
|  | ||
Uh oh!
There was an error while loading. Please reload this page.