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

Use venv for Python development #425

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

hemildesai
Copy link

Signed-off-by: hemildesai <hemil.desai10@gmail.com>
Signed-off-by: hemildesai <hemil.desai10@gmail.com>
Signed-off-by: hemildesai <hemil.desai10@gmail.com>
Signed-off-by: hemildesai <hemil.desai10@gmail.com>
Signed-off-by: hemildesai <hemil.desai10@gmail.com>
Signed-off-by: hemildesai <hemil.desai10@gmail.com>
@hemildesai
Copy link
Author

Not sure why CI / Test (Python) (3.6, ubuntu-20.04) is failing. The error seems to be No matching distribution found for tensorflow-cpu==2.4.0 (from -r requirements-test.txt (line 4)).

@bfirsh
Copy link
Member

bfirsh commented Dec 24, 2020

Yeah, super weird isn't it. I restarted the test a couple of times so it's a real failure.

Maybe it's using a different version of pip because it's inside a virtualenv? I'm trying to figure out what might be different in this installation.

@bfirsh
Copy link
Member

bfirsh commented Dec 24, 2020

Thanks so much for the contribution btw! I'll do some poking around later today if I get time...

@hemildesai
Copy link
Author

hemildesai commented Dec 24, 2020

Cool, thanks for looking into it. Let me know if I can help.

@hemildesai
Copy link
Author

@bfirsh it looks like the pip version is the culprit - tensorflow/tensorflow#39130 (comment)

Signed-off-by: hemildesai <hemil.desai10@gmail.com>
@bfirsh
Copy link
Member

bfirsh commented Dec 24, 2020

Hah, oh god. Nice find. 🙄

@bfirsh
Copy link
Member

bfirsh commented Dec 24, 2020

Several times during this project we've thought "ok we need to burn all Python packaging to the ground and create a new packaging system" but then reminded ourselves we should keep our eyes on the ball, haha. This is also why most of the codebase is in Go -- so we don't have to deal with Python's godawful packaging/dependencies. cc @dscape

This reverts commit b36645f.

Signed-off-by: hemildesai <hemil.desai10@gmail.com>
Signed-off-by: hemildesai <hemil.desai10@gmail.com>
@hemildesai
Copy link
Author

Yeah, sometimes with issues like this, it can be quite frustrating.

@hemildesai
Copy link
Author

Hey @bfirsh, let me know if this is good to go or if there are any changes required.

@bfirsh
Copy link
Member

bfirsh commented Jan 7, 2021

Thanks so much @hemildesai! We're just in the process of shipping a release right now so I don't want to change up the development environment while we do that. But, as soon as we've shipped it, this is first thing I'm taking a look at. :)

@bfirsh
Copy link
Member

bfirsh commented Feb 15, 2021

Sorry for being slow here -- I had a quick look at this the other day. I think it's a bit confusing that the Python library is installed locally in a virtualenv, but the binary is still installed globally. That'll make it easy to get into a situation where different versions installed in two different places. I wonder if we can install the binary inside the venv in development?

@hemildesai
Copy link
Author

@bfirsh Sure. Can you point me to where the binary is installed so that I can change it to be installed in the venv?

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.

None yet

2 participants