search

miykael / gif_your_nifti

How to create fancy GIFs from an MRI brain image
BSD 3-Clause "New" or "Revised" License
118 stars 31 forks source link

Request: `setup.py` pulling requirements from `requirements.txt` #14

Closed jamesbraza closed 1 year ago

jamesbraza commented 1 year ago

setuptools>=62.6 supports install_requires coming from a requirements.txt.

It would be good to sync up requirements.txt with setup.py's install_requires using this.

miykael commented 1 year ago

I agree. This was one of my earlier projects and I definitely missed a lot of opportunities to do it right (e.g. fixing package versions). Feel free to send a PR with regards to this issue as well, happy to integrate it asap.

jamesbraza commented 1 year ago

I see requirements.txt has scikit-image, whereas setup.py doesn't require it.

I was thinking about this last night, what is requirements.txt used for, is it development? Also, should the min versions shown in requirements.txt be propagated to setup.py?

What do you think of requirements.txt editably installing setup.py, such that it becomes:

-e .
scikit-image>=0.13.0
miykael commented 1 year ago

I think I included requirements.txt for those who are used to install dependencies like this (which was me as well back then), plus because it can be used as a quick lookup if you explore the repo. But given that the setup.py is not even using this information I would say currently something is obsolete. So either the setup.py should load information from the requirements.txt or that file can be deleted.

Not even the Dockerfile is using the information from requirements.txt... Definitely not my best work :-)