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

tensorflow should be a main dependency (instead of a dev dependency) #261

Closed
metasyn opened this issue Jan 5, 2022 · 4 comments · Fixed by #262
Closed

tensorflow should be a main dependency (instead of a dev dependency) #261

metasyn opened this issue Jan 5, 2022 · 4 comments · Fixed by #262

Comments

@metasyn
Copy link
Contributor

metasyn commented Jan 5, 2022

I am trying to use scikeras in a poetry based project. I ran

poetry add scikeras

which worked. At runtime, I got an error saying that tensorflow 2.7.0 was needed. This should've been caught at the time I added the package during poetry. The only reason, from my understanding that it wasn't was because tensorflow is a dev dependency, rather than a real dependency, for poetry.

@adriangb
Copy link
Owner

adriangb commented Jan 5, 2022

This is a known issue, and stems from a limitation of how TensorFlow packages itself. The TLDR is that there are other versions of tensor flow like tensorflow-cpu, so if we specified tensorflow as a hard dependency users who want to use tensorflow-cpu wouldn't be able to install SciKeras. For more background, see #221.

@metasyn
Copy link
Contributor Author

metasyn commented Jan 5, 2022

Ah, I see.

#221 (comment)

I think option 1 here is a bit cleaner, but makes sense to go either way to solve the problem. No worries if you just want to close this, I should've read the readme closer. Thanks for the cool package :)

@adriangb
Copy link
Owner

adriangb commented Jan 5, 2022

Yeah I guess we can make TensorFlow an optional dependency and the update everywhere in the documentation to use scikeras[tensorflow] so that at least users copy pasting from the readme won't get caught off guard that easily. A PR for something like this would be gladly accepted :)

@metasyn
Copy link
Contributor Author

metasyn commented Jan 5, 2022

I see. What do you think of adding both?

poetry add --optional tensorflow==2.7.0
poetry add --optional tensorflow-cpu==2.7.0

?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants