Add DataFrame.map to dask.DataFrame API
#10246
Conversation
There was a problem hiding this comment.
Thanks for handling this @phofl. Overall looks good -- just left a couple of small suggestions
dask/dataframe/core.py
Outdated
| @@ -5932,6 +5932,10 @@ def apply( | |||
| def applymap(self, func, meta="__no_default__"): | |||
| return elemwise(M.applymap, self, func, meta=meta) | |||
|
|
|||
| @derived_from(pd.DataFrame) | |||
| def map(self, func, meta="__no_default__"): | |||
There was a problem hiding this comment.
Let's use the no_default variable here instead. If you could update applymap above as well, that would be great (though I realize this pre-dates this PR)
| with pytest.raises( | ||
| NotImplementedError, match="Base package doesn't support 'map'." | ||
| ): |
There was a problem hiding this comment.
This is coming from when we try to get the pd.DataFrame.map docstring in the @derived_from(pd.DataFrame) decorator. It's nice that we get a NotImplementedError error, but something along the lines of (I've not tested / linted the snippet below)
if not PANDAS_GT_210:
raise NotImplementedError(
"DataFrame.map requires pandas>=2.1.0, but pandas=X.Y.Z is installed."
)would, I think, be a bit more clear for users and devs
There was a problem hiding this comment.
Yeah I had this initially :)
Changed, and removed the derived_from.
There was a problem hiding this comment.
Thanks @phofl! Just fixed a merge conflict + included a small fixup -- this looks good to go now 👍 Will merge when CI finishes
pre-commit run --all-files