DEPR/BUG: fix Series.argmin/max #16964
Conversation
|
|
||
| pytest.raises(ValueError, frame.argmin, axis=2) | ||
|
|
||
| def test_argmax(self): |
There was a problem hiding this comment.
These new functions were written to follow the existing structure of the idxmin and idxmax functions. Would you like me to parametrize those functions as well so as to not introduce an inconsistency in the testing methodology?
There was a problem hiding this comment.
No, he's referring to the fact that you were using nested for-loops for testing. You can avoid those by parametrizing as illustrated here.
Codecov Report
@@ Coverage Diff @@
## master #16964 +/- ##
==========================================
+ Coverage 90.98% 90.99% +<.01%
==========================================
Files 161 161
Lines 49288 49300 +12
==========================================
+ Hits 44846 44861 +15
+ Misses 4442 4439 -3
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #16964 +/- ##
==========================================
- Coverage 90.99% 90.97% -0.02%
==========================================
Files 161 161
Lines 49290 49302 +12
==========================================
+ Hits 44851 44854 +3
- Misses 4439 4448 +9
Continue to review full report at Codecov.
|
doc/source/whatsnew/v0.21.0.txt
Outdated
| @@ -116,6 +116,8 @@ Other API Changes | |||
| Deprecations | |||
| ~~~~~~~~~~~~ | |||
| - :func:`read_excel()` has deprecated ``sheetname`` in favor of ``sheet_name`` for consistency with ``.to_excel()`` (:issue:`10559`). | |||
| - :method:`Series.argmax` has been deprecated in favor of :method:`Series.idxmax` (:issue:`16830`) | |||
| - :method:`Series.argmin` has been deprecated in favor of :method:`Series.idxmin` (:issue:`16830`) | |||
There was a problem hiding this comment.
Remove these. Refrain from writing anything in the whatsnew until we can agree which version your changes will be applied.
|
Tagging this for |
lphk92
force-pushed
the
issue-16830-part2
branch
from
a71b040
to
5f628c2
Compare
|
I would rather open an issue that references this and mark it for 1.0. Then the PR can be resurrected at that time. |
git diff upstream/master -u -- "*.py" | flake8 --diffThis PR actually implements the proper behavior for argmax() and argmin() for both Series and DataFrame. We will have to wait to rebase this onto master until after a new release.