atomistic-machine-learning / schnetpack

SchNetPack - Deep Neural Networks for Atomistic Systems
Other
791 stars 215 forks source link

requirements.txt is linked to setup.py #588

Closed jnsLs closed 11 months ago

jnsLs commented 12 months ago

what do you think of doing it this way. pip install -r requirements.txt will basically run setup.py now

alternatively you could parse the requirements.txt file in setup.py but that is not recommended

stefaanhessmann commented 12 months ago

I think this might be problematic because of PyPI. I am not sure what command they use for the installation, but I think they will run setup.py and not pip install -r requirements.txt.

How about we add this to setup.py:

with open('requirements.txt') as f:
    requirements = [line.strip() for line in f.readlines()]

and then use package_requires=requirements in setup.py? I just tested it and it seems to work well

jnsLs commented 12 months ago

I don't think PyPI would be a problem since the setup.py contains the dependencies.

Basically now you can either run pip install -r requirements.txt or pip install .

both works. The only thing that is not so nice, is that 'pip install -r requirements.txt' actually installs SchNetPack (incl. dependencies) and not only the dependencies.

I did it this way because they say its not recommended to parse the requirements.txt file in setup.py: https://stackoverflow.com/questions/14399534/reference-requirements-txt-for-the-install-requires-kwarg-in-setuptools-setup-py

However, I am also fine with what you propose. As long as we keep the requirement file clean it should not be a problem.