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.

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

[modin] Append to path to avoid namespace collision on development branches #3621

Merged
merged 1 commit into from
Dec 24, 2018

Conversation

devin-petersohn
Copy link
Member

What do these changes do?

Allows development branches to run Modin.

Related issue number

Resolves #3611

@robertnishihara
Copy link
Collaborator

Hm, this is the behavior we want if Modin is imported first, but not the behavior we want if Ray is imported first.

I guess the problem comes from the fact that on the workers, Ray is always imported first.

@richardliaw
Copy link
Contributor

Would a soft import help here too? ie, try import modin and if not found, then append to path?

@robertnishihara
Copy link
Collaborator

Ideally we'd have two different behaviors.

  1. If ray is imported first, then import modin will only find the copy of Modin that is shipped with Ray (@richardliaw I think your proposal violates this).
  2. If modin is imported first, then we only find the copy of modin that is first on the PYTHONPATH.

Unfortunately I don't see a good way to do these both simultaneously.

@richardliaw
Copy link
Contributor

Oh; sorry I guess I'm missing context. When do you want the first behavior? Why not have ray.modin separate from modin (which holds its own in site-packages)?

@devin-petersohn devin-petersohn changed the title Append to path to avoid namespace collision on development branches [modin] Append to path to avoid namespace collision on development branches Dec 23, 2018
@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/10334/
Test FAILed.

@devin-petersohn
Copy link
Member Author

@richardliaw - The goal was to make this similar to how pyarrow is handled in Ray. The challenge is that Modin depends on Ray. It can cause the version of Modin being imported on the workers to mismatch the version on the driver, which is especially problematic for releases where we have made significant changes to the code that runs in Ray.

@robertnishihara robertnishihara merged commit c13b268 into ray-project:master Dec 24, 2018
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.

[modin] Importing Modin before Ray can sometimes cause ImportError
4 participants