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

simplify requirements.txt #28824

Merged
merged 15 commits into from
Sep 18, 2024
Merged

simplify requirements.txt #28824

merged 15 commits into from
Sep 18, 2024

Conversation

Sam-Armstrong
Copy link
Contributor

@Sam-Armstrong Sam-Armstrong commented Sep 17, 2024

Remove unnecessary dependencies from the requirements.txt file.

@Sam-Armstrong
Copy link
Contributor Author

@hmahmood24 @YushaArif99 am I ok to merge this? seems to be fine with local testing, but I think the only way to test it out properly will be as part of the 'full ivy test from fresh dev perspective' which we can do soon.

Copy link
Contributor

@YushaArif99 YushaArif99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm at a first glance! But i guess we'll need to do some e2e testing to verify everything works smoothly from an end user's perspective.

Copy link
Contributor

@hmahmood24 hmahmood24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Sam-Armstrong Thanks for working on this. The first question/observation that comes to mind is that requirements.txt is supposed to represent the minimal requirements in order to use ivy out of the box without minimal effort and since we are launching ivy with the purpose of using it as a transpiler with support for going from torch to tensorflow, do you think it makes sense to have these deps in the regular requirements.txt file because if someone does pip install ivy, we'd only install numpy and that's no good (it was good when it was to be used as a framework but is useless if we are marketing ivy as a transpiler with no support for transpiling to/fro numpy at launch). If you look at transformers for example, they have tensorflow, torch, jax and numpy all listed down directly in their setup.py rather than have optional requirements files that the users themselves have to install regardless of whether they're installing from source or from pip

@hmahmood24
Copy link
Contributor

Also would be good to pin the versions as part of this PR @Sam-Armstrong

@Sam-Armstrong
Copy link
Contributor Author

I'll make a new PR for pinning the frameworks, so we can check we agree on the pinned versions.

@Sam-Armstrong Sam-Armstrong merged commit bcde696 into main Sep 18, 2024
3 of 7 checks passed
@Sam-Armstrong Sam-Armstrong deleted the requirements branch September 18, 2024 12:35
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.

3 participants