Updated qcut for Float64DType Issue #40730 #40969
Conversation
|
@github-actions pre-commit |
|
Thanks for the pr @taytzehao! Can you please add tests that fail before this fix, but pass after? I think |
mzeitlin11
added
Bug
cut
|
Hello @taytzehao! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2021-04-19 09:51:16 UTC |
doc/source/whatsnew/v1.3.0.rst
Outdated
| @@ -695,7 +695,7 @@ Conversion | |||
| - Bug in :class:`Index` construction silently ignoring a passed ``dtype`` when the data cannot be cast to that dtype (:issue:`21311`) | |||
| - Bug in :meth:`StringArray.astype` falling back to numpy and raising when converting to ``dtype='categorical'`` (:issue:`40450`) | |||
| - Bug in :class:`DataFrame` construction with a dictionary containing an arraylike with ``ExtensionDtype`` and ``copy=True`` failing to make a copy (:issue:`38939`) | |||
| - | |||
| - Bug in :func:`_coerce_to_type` failing to convert ``Float64DType`` input into numpy array (:issue:`40730`) | |||
There was a problem hiding this comment.
Whatsnew entries should reference behavior that users see. So instead of an internal function, it should mention the effect on qcut, something like Bug in :meth:pandas.qcut raising for nullable float types
There was a problem hiding this comment.
Got it. Will resolve it accordingly.
pandas/tests/reshape/test_qcut.py
Outdated
| pd.qcut(series, 2) | ||
| except: | ||
| Fail_string = Data_type_string + " is not supported" | ||
| pytest.fail(msg=Fail_string) |
There was a problem hiding this comment.
What are you trying to test here? If you're trying to test that something works, this try/except structure is not needed, if it actually raises the test will just fail as desired by default.
It might be helpful to try to follow examples from above - construct result from using qcut, then compare to an expected result that you hardcode.
I'd recommend just folding into the test above - you could just parameterize the test above by any_nullable_numeric_dtype type instead of any_nullable_int_dtype rather than adding a new test.
|
@github-actions pre-commit |
|
@mzeitlin11 , let me know what you think |
|
thanks @taytzehao nice way to fix! |
This PR is used to address #40730 . qcut is now able to support both intdtype and floatdtype