Conversation
(sorry for the hack, this was useful for std/var in dask-gdf)
These don't do anything currently, and are silently ignored
|
Can one of the admins verify this patch? |
|
LGTM |
| if other == 2: | ||
| return self * self | ||
| else: | ||
| return NotImplemented |
There was a problem hiding this comment.
This is janky. Is there a GPU implementation somewhere that we can use?
kkraus14
added
2 - In Progress
|
add to whitelist |
mrocklin
force-pushed
the
dask-gdf-fixes
branch
from
84b14c4 to
d1bc16e
Compare
pygdf/series.py
Outdated
| data.columns = ['x'] | ||
| data = DataFrame.from_pandas(data) | ||
| data = data['x'] | ||
| data.name = name |
There was a problem hiding this comment.
This feels like a hacky workaround where we should address the issue(s) instead.
There was a problem hiding this comment.
Fair point. I'll dive through and figure out how to construct a Series object manually in the morning. Any pointers would be welcome.
There was a problem hiding this comment.
Currently a pygdf.Series can be constructed from a pandas.Series just via normal __init__ which ends up hitting this clause: https://github.com/gpuopenanalytics/pygdf/blob/master/pygdf/columnops.py#L183
So the Pandas Series is casted to a numpy array. I think you could add a check for if it's an instance of a Pandas Series and set the name if it is and then call the columnops.as_column function.
We were listing all locals in the file, so it doesn't seem to accomplish much. It does however require people editing this file to make two changes rather than one.
|
Changes look good to me thus far, some of the binaryops will require a bit of refactoring once new binaryops land in libgdf, but that's to be expected. |
If this seems mergable to you without added burden then I would not be against merging it. I think that most of this is defensible regardless of what happens. |
4 participants
This PR includes a few small fixes that were useful in getting dask-gdf to inherit nicely from dask.dataframe. In doing so we found that the PyGDF code needed to match pandas slightly more in a few cases.
This is a work in progress. I would not merge this. So far it's up for discussion only.