Skip to content

Conversation

sharkipelago
Copy link
Contributor

I changed the behavior or series.round() to act like series.map(round) if the dtype is object as discussed in #61682. I put this in a new PR as the original intent of the other PR was to change DataFrame behavior. I'm unsure if the way I approached it is a good way to make the change though because I saw in core/internals/blocks.py that Block's round() method is intended to raise an error for dtype of object. So I wasn't sure I should approach this by updated the Block class instead. Any feedback is appreciated!

return self._constructor(
new_values, index=self.index, copy=False
).__finalize__(self, method="map")
except TypeError as e:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick e->err. try to avoid single-letter variable names

raise TypeError("Expected numeric dtype, got object instead.")
try:
round_func = functools.partial(round, ndigits=decimals)
new_values = self._map_values(round_func)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only this line needs to go inside the try/except

@jbrockmendel
Copy link
Member

that Block's round() method is intended to raise an error for dtype of object

In another PR you're going to make DataFrame do the same thing this does, right? If so, you'll need to change the Block behavior.

ser = Series([0.232], dtype="object")
expected = Series([0.2])
tm.assert_series_equal(ser.round(1), expected)
ser2 = Series(["bar"], dtype="object")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you make this a separate test

# GH#61206, GH#62174
ser = Series([0.232], dtype="object")
expected = Series([0.2])
tm.assert_series_equal(ser.round(1), expected)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

separate line for result = ser.round(1)

@jbrockmendel
Copy link
Member

i suspect this will also fix/change the DataFrame.round behavior? can you test that and address it in the whatsnew note

Comment on lines 1516 to 1522
round_func = functools.partial(round, ndigits=decimals)
mapper = functools.partial(algos.map_array, mapper=round_func)
try:
if self.values.ndim == 1:
values = algos.map_array(self.values, round_func)
else:
values = np.apply_along_axis(mapper, 0, self.values)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely feels like there is a better way to account for 2d blocks but I couldn't think of a better way. Part of that was I couldn't find where in lib.map_infer() was defined (from the return statement in algos.map_array()) and thus didn't know why algos.map_array wasn't working for the 2d ndarray.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

algos.map_array(self.values.ravel(), round_func).reshape(self.values.shape)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohh okay makes sense, thank you

@final
def __repr__(self) -> str:
# don't want to print out all of the items here
# don't want to out all of the items here
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, thanks for catching that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants