-
-
Notifications
You must be signed in to change notification settings - Fork 18k
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
ENH: implement FloatingArray.round() #38866
Changes from all commits
1df9bf7
192ebad
6403026
253d279
3e0d605
fe19597
b557a50
85e6d4f
f79c0cd
d1cd6d8
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 |
---|---|---|
@@ -1,10 +1,12 @@ | ||
import datetime | ||
from typing import TYPE_CHECKING, Union | ||
from typing import TYPE_CHECKING, Callable, Union | ||
|
||
import numpy as np | ||
|
||
from pandas._libs import Timedelta, missing as libmissing | ||
from pandas.compat.numpy import function as nv | ||
from pandas.errors import AbstractMethodError | ||
from pandas.util._decorators import doc | ||
|
||
from pandas.core.dtypes.common import ( | ||
is_float, | ||
|
@@ -56,6 +58,32 @@ def __from_arrow__( | |
return array_class._concat_same_type(results) | ||
|
||
|
||
_round_doc = """ | ||
Round each value in NumericArray a to the given number of decimals. | ||
|
||
Parameters | ||
---------- | ||
decimals : int, default 0 | ||
Number of decimal places to round to. If decimals is negative, | ||
it specifies the number of positions to the left of the decimal point. | ||
*args, **kwargs | ||
Additional arguments and keywords have no effect but might be | ||
accepted for compatibility with NumPy. | ||
|
||
Returns | ||
------- | ||
NumericArray | ||
Rounded values of the NumericArray. | ||
|
||
See Also | ||
-------- | ||
numpy.around : Round values of an np.array. | ||
DataFrame.round : Round values of a DataFrame. | ||
Series.round : Round values of a Series. | ||
|
||
""" | ||
|
||
|
||
class NumericArray(BaseMaskedArray): | ||
""" | ||
Base class for IntegerArray and FloatingArray. | ||
|
@@ -130,3 +158,16 @@ def _arith_method(self, other, op): | |
) | ||
|
||
return self._maybe_mask_result(result, mask, other, op_name) | ||
|
||
def _apply(self, func: Callable, **kwargs) -> "NumericArray": | ||
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. cc @jbrockmendel @jorisvandenbossche shall we make this more general? (e.g. on base.py) 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. (I would for this PR leave it here in order to do the minimal to actually implement the |
||
values = self._data[~self._mask] | ||
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. can you add a doc-string 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. I don't think you actually need to subset the Of course, if many values are masked, we might be calculating |
||
values = np.round(values, **kwargs) | ||
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. should be func :-> |
||
|
||
data = np.zeros(self._data.shape) | ||
data[~self._mask] = values | ||
return type(self)(data, self._mask) | ||
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. The mask needs to be copied I think? (result should not share a mask with the original array, because otherwise editing one can modify the other. We should probably also test this) 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. That's a good point (and actually the same bug exists already in my implementation of |
||
|
||
@doc(_round_doc) | ||
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. The docstring can be moved inline? |
||
def round(self, decimals: int = 0, *args, **kwargs) -> "NumericArray": | ||
nv.validate_round(args, kwargs) | ||
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. If we accept args/kwargs here and validate them, then we should also test this (eg doing |
||
return self._apply(np.round, decimals=decimals, **kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NumericArray is not public, and thus we shouldn't mention it in the whatsnew notes. You can say something about "round() being enabled for the nullable integer and floating dtypes"