COMPAT: Iteration should always yield a python scalar #17491
Conversation
Codecov Report
@@ Coverage Diff @@
## master #17491 +/- ##
==========================================
+ Coverage 91.15% 91.15% +<.01%
==========================================
Files 163 163
Lines 49534 49540 +6
==========================================
+ Hits 45153 45160 +7
+ Misses 4381 4380 -1
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #17491 +/- ##
==========================================
+ Coverage 91.15% 91.16% +0.01%
==========================================
Files 163 163
Lines 49534 49543 +9
==========================================
+ Hits 45153 45168 +15
+ Misses 4381 4375 -6
Continue to review full report at Codecov.
|
c0fd989 to
6a02e4f
Compare
|
|
||
| Previously: | ||
|
|
||
| .. code-block:: python |
|
|
||
| .. ipython:: python | ||
|
|
||
| s = Series([1, 2, 3]) |
| Iteration of Series/Index will now return python scalars | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| Previously, when using certain iteration methods for a ``Series`` with dtype ``int`` or ``float``, you would receive a ``numpy`` scalar, e.g. a ``np.int64``, rather than a python ``int``. Issue (:issue:`10904`) corrected this for ``Series.tolist()`` and ``list(Series)``. This change makes all iteration methods consistent, in particular, for ``__iter__()`` and ``.map()``; note that this only affect int/float dtypes. (:issue:`13236`, :issue:`13258`, :issue:`14216`). |
There was a problem hiding this comment.
this only affect -> this only affects
|
|
||
| Previously: | ||
|
|
||
| .. code-block:: python |
| @@ -884,6 +890,21 @@ def argmin(self, axis=None): | |||
| """ | |||
| return nanops.nanargmin(self.values) | |||
|
|
|||
| def tolist(self): | |||
| """ | |||
| return a list of the values; box to scalars | |||
There was a problem hiding this comment.
Can you put a bit more explanation that python scalar types are returned?
| if is_datetimelike(self): | ||
| return (_maybe_box_datetimelike(x) for x in self._values) | ||
| else: | ||
| return iter(self._values.tolist()) |
There was a problem hiding this comment.
For the tolist implementation, this seems a bit double work: values are converted to list, then iterateted over, and then again converted to list.
| @@ -253,6 +253,10 @@ def get_values(self): | |||
| """ return the underlying data as an ndarray """ | |||
| return self._data.get_values() | |||
|
|
|||
| def __iter__(self): | |||
| """ iterate like Categorical """ | |||
| return self._data.__iter__() | |||
There was a problem hiding this comment.
this feels not clean. The tolist of Categorical should already ensure this?
| # gh-10904 | ||
| # gh-13258 | ||
| # coerce iteration to underlying python / pandas types | ||
| s = typ([1], dtype=dtype) |
There was a problem hiding this comment.
I would make a separate construction for object/category, because the test is not ensuring this is correct. For example on master, a categorical series of integers will box to np.int64, but a np.int64 scalar passes the ininstance(.., object) test
| Timestamp('2000-12-31')]) | ||
|
|
||
| result = method(i)[0] | ||
| assert isinstance(result, Timestamp) |
There was a problem hiding this comment.
can you add test for Series.iteritems and DataFrame.itertuples/iterrows as well?
There was a problem hiding this comment.
these are in tests/frame/test_api.py already
There was a problem hiding this comment.
Yes, but you said this PR changed the behaviour of itertuples ? (#13468 (comment)) Then we should have a test for that?
There was a problem hiding this comment.
and that's all well tested see
frame/test_api
There was a problem hiding this comment.
ah, missed that one-line change in existing test in the diff. Thanks for the clarification!
xref #10904
closes #13236
closes #13258
xref #14216