From 5c6837f1d2460a988fef83f6905948b55b0cab6d Mon Sep 17 00:00:00 2001 From: TimJ Date: Thu, 27 Jun 2019 16:39:48 +0100 Subject: [PATCH 1/5] BUG/TST: :meth:`DataFrame.fillna` ``limit`` checks and tests Reversed the order of returning and performing checks on `limit` in fillna method in blocks.py so that an ValueError is raised before returning. --- doc/source/whatsnew/v0.25.0.rst | 3 ++- pandas/core/internals/blocks.py | 12 ++++++------ pandas/tests/frame/test_missing.py | 16 ++++++++++++++++ 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/doc/source/whatsnew/v0.25.0.rst b/doc/source/whatsnew/v0.25.0.rst index 18a3785867714..09e77d2c5ad20 100644 --- a/doc/source/whatsnew/v0.25.0.rst +++ b/doc/source/whatsnew/v0.25.0.rst @@ -685,7 +685,8 @@ Missing - Fixed misleading exception message in :meth:`Series.interpolate` if argument ``order`` is required, but omitted (:issue:`10633`, :issue:`24014`). - Fixed class type displayed in exception message in :meth:`DataFrame.dropna` if invalid ``axis`` parameter passed (:issue:`25555`) -- +- Fixed not throwing ValueError in :meth:`DataFrame.fillna` when called with a negative ``limit`` parameter value (:issue:`27042`) +- MultiIndex ^^^^^^^^^^ diff --git a/pandas/core/internals/blocks.py b/pandas/core/internals/blocks.py index c6be56df7ae0c..0b2af9391784c 100644 --- a/pandas/core/internals/blocks.py +++ b/pandas/core/internals/blocks.py @@ -344,12 +344,6 @@ def fillna(self, value, limit=None, inplace=False, downcast=None): """ inplace = validate_bool_kwarg(inplace, 'inplace') - if not self._can_hold_na: - if inplace: - return self - else: - return self.copy() - mask = isna(self.values) if limit is not None: if not is_integer(limit): @@ -361,6 +355,12 @@ def fillna(self, value, limit=None, inplace=False, downcast=None): "is currently limited to 2") mask[mask.cumsum(self.ndim - 1) > limit] = False + if not self._can_hold_na: + if inplace: + return self + else: + return self.copy() + # fillna, but if we cannot coerce, then try again as an ObjectBlock try: values, _ = self._try_coerce_args(self.values, value) diff --git a/pandas/tests/frame/test_missing.py b/pandas/tests/frame/test_missing.py index a1dbeba364240..c72951ac4cdfa 100644 --- a/pandas/tests/frame/test_missing.py +++ b/pandas/tests/frame/test_missing.py @@ -516,6 +516,22 @@ def test_fillna_skip_certain_blocks(self): # it works! df.fillna(np.nan) + @pytest.mark.parametrize("type", [int, float]) + def test_fillna_positive_limit(self, type): + df = DataFrame(np.random.randn(10, 4)).astype(type) + + msg = "Limit must be greater than 0" + with pytest.raises(ValueError, match=msg): + df.fillna(0, limit=-5) + + @pytest.mark.parametrize("type", [int, float]) + def test_fillna_integer_limit(self, type): + df = DataFrame(np.random.randn(10, 4)).astype(type) + + msg = "Limit must be an integer" + with pytest.raises(ValueError, match=msg): + df.fillna(0, limit=0.5) + def test_fillna_inplace(self): df = DataFrame(np.random.randn(10, 4)) df[1][:4] = np.nan From c6dc6d9f2decc86ffdc540648083ca3869210aee Mon Sep 17 00:00:00 2001 From: TimJ Date: Thu, 27 Jun 2019 16:55:29 +0100 Subject: [PATCH 2/5] Update to whatsnew entry --- doc/source/whatsnew/v0.25.0.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v0.25.0.rst b/doc/source/whatsnew/v0.25.0.rst index 09e77d2c5ad20..b412e94577d1e 100644 --- a/doc/source/whatsnew/v0.25.0.rst +++ b/doc/source/whatsnew/v0.25.0.rst @@ -685,7 +685,7 @@ Missing - Fixed misleading exception message in :meth:`Series.interpolate` if argument ``order`` is required, but omitted (:issue:`10633`, :issue:`24014`). - Fixed class type displayed in exception message in :meth:`DataFrame.dropna` if invalid ``axis`` parameter passed (:issue:`25555`) -- Fixed not throwing ValueError in :meth:`DataFrame.fillna` when called with a negative ``limit`` parameter value (:issue:`27042`) +- Fixed not throwing `ValueError` in :meth:`DataFrame.fillna` when called with a negative ``limit`` parameter value (:issue:`27042`) - MultiIndex From 91a4329ef73cb97b00b59d45bd3a62f0116ddd0a Mon Sep 17 00:00:00 2001 From: TimJ Date: Thu, 27 Jun 2019 16:57:10 +0100 Subject: [PATCH 3/5] Update to whatsnew entry (again) --- doc/source/whatsnew/v0.25.0.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v0.25.0.rst b/doc/source/whatsnew/v0.25.0.rst index b412e94577d1e..d11e72c3f7168 100644 --- a/doc/source/whatsnew/v0.25.0.rst +++ b/doc/source/whatsnew/v0.25.0.rst @@ -685,7 +685,7 @@ Missing - Fixed misleading exception message in :meth:`Series.interpolate` if argument ``order`` is required, but omitted (:issue:`10633`, :issue:`24014`). - Fixed class type displayed in exception message in :meth:`DataFrame.dropna` if invalid ``axis`` parameter passed (:issue:`25555`) -- Fixed not throwing `ValueError` in :meth:`DataFrame.fillna` when called with a negative ``limit`` parameter value (:issue:`27042`) +- Fixed not throwing ``ValueError`` in :meth:`DataFrame.fillna` when called with a negative ``limit`` parameter value (:issue:`27042`) - MultiIndex From 800036ed1fdc431e5162c1f27172d6ed4359f899 Mon Sep 17 00:00:00 2001 From: TimJ Date: Thu, 27 Jun 2019 17:29:12 +0100 Subject: [PATCH 4/5] Removed whitespace --- doc/source/whatsnew/v0.25.0.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v0.25.0.rst b/doc/source/whatsnew/v0.25.0.rst index d11e72c3f7168..52e92f9bb0d83 100644 --- a/doc/source/whatsnew/v0.25.0.rst +++ b/doc/source/whatsnew/v0.25.0.rst @@ -686,7 +686,7 @@ Missing - Fixed misleading exception message in :meth:`Series.interpolate` if argument ``order`` is required, but omitted (:issue:`10633`, :issue:`24014`). - Fixed class type displayed in exception message in :meth:`DataFrame.dropna` if invalid ``axis`` parameter passed (:issue:`25555`) - Fixed not throwing ``ValueError`` in :meth:`DataFrame.fillna` when called with a negative ``limit`` parameter value (:issue:`27042`) -- +- MultiIndex ^^^^^^^^^^ From b5dc82e246be248d55dfe11f1f1f4d067faba43b Mon Sep 17 00:00:00 2001 From: Inevitable-Marzipan <43890311+Inevitable-Marzipan@users.noreply.github.com> Date: Thu, 27 Jun 2019 18:24:23 +0100 Subject: [PATCH 5/5] Update doc/source/whatsnew/v0.25.0.rst Co-Authored-By: William Ayd --- doc/source/whatsnew/v0.25.0.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v0.25.0.rst b/doc/source/whatsnew/v0.25.0.rst index 52e92f9bb0d83..21ab5128e061a 100644 --- a/doc/source/whatsnew/v0.25.0.rst +++ b/doc/source/whatsnew/v0.25.0.rst @@ -685,7 +685,7 @@ Missing - Fixed misleading exception message in :meth:`Series.interpolate` if argument ``order`` is required, but omitted (:issue:`10633`, :issue:`24014`). - Fixed class type displayed in exception message in :meth:`DataFrame.dropna` if invalid ``axis`` parameter passed (:issue:`25555`) -- Fixed not throwing ``ValueError`` in :meth:`DataFrame.fillna` when called with a negative ``limit`` parameter value (:issue:`27042`) +- A ``ValueError`` will now be thrown by :meth:`DataFrame.fillna` when ``limit`` is not a positive integer (:issue:`27042`) - MultiIndex