-
Notifications
You must be signed in to change notification settings - Fork 31.9k
[setup] update/add setup targets #8076
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
Conversation
sgugger
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for humoring us Stas. This looks good to me!
|
|
||
| extras["flax"] = ["jaxlib==0.1.55", "jax>=0.2.0", "flax==0.2.2"] | ||
| if os.name == "nt": # windows | ||
| extras["flax"] = [] # jax is not supported on windows |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we define extras["retrieval"] before this line with just "datasets" and append "faiss-cpu" here (it only works on Windows too).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it only works on non-windows (linux + osx), but good comment nonetheless!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes non-Windows, sorry ;-)
LysandreJik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, thanks @stas00!
|
|
||
| extras["flax"] = ["jaxlib==0.1.55", "jax>=0.2.0", "flax==0.2.2"] | ||
| if os.name == "nt": # windows | ||
| extras["flax"] = [] # jax is not supported on windows |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it only works on non-windows (linux + osx), but good comment nonetheless!
|
Guys, since this is not code but a bunch of definitions please make direct suggestions that can be merged. Thank you. I originally just wanted to add |
|
I can't make a suggestion that moves stuff around in the file. I can push a commit to your branch if you want, but that's the best I can do. |
|
I understand. I just don't know to work in this fashion. If you give me a spec I can work to implement it. Otherwise let's just merge this and subsequent PRs can do further improvements. |
|
Sure, I'll add my comments in a separate PR. Thanks for doing this one! |
updating pip targets
tokenizerdocstodev, since we need to have the tools to runmake docsflaxtodev, since we need to have the libs to run flax tests - except when on windows - it skips it thenallup-to-date@sgugger