Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Dataframe] Change pandas and ray.dataframe imports #1942

Merged
merged 4 commits into from
Jun 15, 2018
Merged

[Dataframe] Change pandas and ray.dataframe imports #1942

merged 4 commits into from
Jun 15, 2018

Conversation

kunalgosar
Copy link
Contributor

Changes imports to:

import pandas
import ray.dataframe as pd

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/5042/
Test FAILed.

@robertnishihara
Copy link
Collaborator

I can see why a user would want to do this (to simplify porting code), but does it make sense in our own codebase?

I could see this going either way, but the previous approach seems clearer to me.

@kunalgosar
Copy link
Contributor Author

This is more for consistency, so that we are using the same import internally in the code base as we are recommending externally. It motivates more people to think of this as a drop in replacement.

@robertnishihara
Copy link
Collaborator

Makes sense. I could see this leading to errors in our codebase. E.g., people will accidentally write pd and mean pandas, since we use both.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/5043/
Test PASSed.

@devin-petersohn
Copy link
Member

It's also an attempt at moving away from "Do as I say, not as I do." I think we should use things the way we want users to use them as well.

@robertnishihara
Copy link
Collaborator

Ok, fine with me :)

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/5582/
Test PASSed.

@kunalgosar kunalgosar mentioned this pull request May 24, 2018
Merged
@robertnishihara
Copy link
Collaborator

Is this PR still in progress?

@kunalgosar
Copy link
Contributor Author

Yes, we're waiting on some other PRs to go through to avoid merge conflicts. This refactor can be merged in as a last step before the release.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/6010/
Test FAILed.

@kunalgosar
Copy link
Contributor Author

Jenkins, retest this please

@devin-petersohn
Copy link
Member

Jenkins, retest this please.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/6071/
Test PASSed.

@devin-petersohn devin-petersohn merged commit 8560993 into ray-project:master Jun 15, 2018
royf added a commit to royf/ray that referenced this pull request Jun 22, 2018
@royf
Merge branch 'master' of https://github.com/ray-project/ray
d1a3e44 
* 'master' of https://github.com/ray-project/ray: (157 commits)
  Fix build failure while using make -j1. Issue 2257 (ray-project#2279)
  Cast locator with index type (ray-project#2274)
  fixing zero length partitions (ray-project#2237)
  Make actor handles work in Python mode. (ray-project#2283)
  [xray] Add error table and push error messages to driver through node manager. (ray-project#2256)
  addressing comments (ray-project#2210)
  Re-enable some actor tests. (ray-project#2276)
  Experimental: enable automatic GCS flushing with configurable policy. (ray-project#2266)
  [xray] Sets good object manager defaults. (ray-project#2255)
  [tune] Update Trainable doc to expose interface (ray-project#2272)
  [rllib] Add a simple REST policy server and client example (ray-project#2232)
  [asv] Pushing to s3 (ray-project#2246)
  [rllib] Remove need to pass around registry (ray-project#2250)
  Support multiple availability zones in AWS (fix ray-project#2177) (ray-project#2254)
  [rllib] Add squash_to_range model option (ray-project#2239)
  Mitigate randomly building failure: adding gen_local_scheduler_fbs to raylet lib. (ray-project#2271)
  [rllib] Refactor Multi-GPU for PPO (ray-project#1646)
  [rllib] Envs for vectorized execution, async execution, and policy serving (ray-project#2170)
  [Dataframe] Change pandas and ray.dataframe imports (ray-project#1942)
  [Java] Replace binary rewrite with Remote Lambda Cache (SerdeLambda) (ray-project#2245)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@devin-petersohn devin-petersohn devin-petersohn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kunalgosar @AmplabJenkins @robertnishihara @devin-petersohn