py4dstem / py4DSTEM

GNU General Public License v3.0
206 stars 141 forks source link

setup.py, install_requires: relax dependencies #173

Closed magnunor closed 2 years ago

magnunor commented 3 years ago

Currently, the install requirements is hard-locked to specific versions of the dependencies.

This makes it difficult to install py4dstem together with other libraries.

One solution to this, is to change the requirements from == to >=.

bsavitzky commented 3 years ago

Hey Magnus - yup, this is a great point. When the requirements were looser we were running into some install issues, so pinning all of them was a temporary solution - as you point out, this is not a great long term solution, as it makes compatibility with other packages very difficult

din14970 commented 3 years ago

I'll add to that that it would be helpful to get an idea about which package versions are absolutely necessary for updating the conda-forge package. Currently the conda-forge version is very much out of date, but it is not possible to update because the package versions in the setup.py are mutually incompatible, see this PR. I presume this is why the installation instructions say that python 3.8 is required as this comes with the old pip which did not have a proper dependency resolver and allowed you to create environments with mutually incompatible package versions. In general it would be good to ensure the dependencies in the environment are actually compatible. Conda and the newest pip will do this. Installing with conda-forge in a venv tends to be a easier since users don't have to compile dependencies like numpy on their system, so it would be good to have an updated py4DSTEM in their repos as well. As Magnus said, >= would be helpful, or it would be good to at least have an indication when a dependency must be <=. Of course it would also be helpful if you could help out in maintaining the conda-forge recipe as you know the software best.

din14970 commented 3 years ago

Judging by what the dependency resolver came up with as consistent (see versions on the right, on the left is from setup.py):

py4dstem 0.12.3 has requirement dill==0.3.3, but you have dill 0.3.4.
py4dstem 0.12.3 has requirement h5py==2.10.0, but you have h5py 3.3.0.
py4dstem 0.12.3 has requirement numba==0.49.1, but you have numba 0.53.1.
py4dstem 0.12.3 has requirement numpy==1.19, but you have numpy 1.21.0.
py4dstem 0.12.3 has requirement PyQt5==5.13, but you have pyqt5 5.12.3.
py4dstem 0.12.3 has requirement pyqtgraph==0.11, but you have pyqtgraph 0.12.1.
py4dstem 0.12.3 has requirement qtconsole==4.7.7, but you have qtconsole 5.1.0.
py4dstem 0.12.3 has requirement scikit-image==0.17.2, but you have scikit-image 0.18.1.
py4dstem 0.12.3 has requirement scikit-learn==0.23.2, but you have scikit-learn 0.24.2.
py4dstem 0.12.3 has requirement scipy==1.5.2, but you have scipy 1.6.3.
py4dstem 0.12.3 has requirement tqdm==4.46.1, but you have tqdm 4.61.1.

I would guess that maybe breaking changes in h5py could have caused the biggest issues. Do you know/remember what the main issues were?

alex-rakowski commented 3 years ago

A small point, if NumPy requirement is changed to allow >1.19 the following two lines need updating to np.int32/np.int64. These are the only ones I think are affected by the depreciation of np.int. I quickly checked with grep -r 'np.int' grep -rw 'np.int' and grep -r 'numpy.int' , which should have found all of them.

py4DSTEM/process/preprocess/electroncount.py: bins = np.arange(binmin,binmax,step=step,dtype=np.int) py4DSTEM/process/ptychography/ssb.py: strides = cp.array((np.array(G.strides) / (G.nbytes / G.size)).astype(np.int))