Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Move requirements into setup.py #12

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fralik
Copy link

@fralik fralik commented Mar 23, 2022

  • By moving requirements into setup.py file we make sure that necessary dependencies are installed automatically when user performs pip install mtscomp.

* By moving requirements into setup.py file we make sure that necessary dependencies are installed automatically when user performs `pip install mtscomp`.
@rossant rossant requested review from rossant and oliche March 24, 2022 12:50
Copy link
Member

@oliche oliche left a comment

Choose a reason for hiding this comment

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

Agreed, the setup.py should install requirements !
But it's a good practice to keep the requirements file and read it from the setup as this:

with open('requirements.txt') as f:
    require = [x.strip() for x in f.readlines() if not x.startswith('git+')]
...
install_requires=require,

@fralik
Copy link
Author

fralik commented Mar 30, 2022

OK, can do this. Although, I do not see a real value of a separate requirements file. It is normally used to recreate a development/production environment as a whole, i.e. it would contain pinned version of all libraries (direct dependencies and their subdependencies) installed in an environment. There are tools, like pip-tools, that are able to generate such requirements file from various sources. But since mtscomp may be seen as a library rather than a deployable app, it doesn't make much sense to freeze the environment in a file.

From the code example you provided, why would you want to exclude git+ requirements? Setuptools is able to install those as far as I know.

* Reintroduce requirements files as per request from project maintainers.
@fralik
Copy link
Author

fralik commented Apr 11, 2022

@oliche , implemented your suggestion about using requirements file.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants