SciKit-Surgery / scikit-surgerytutorial01

Our first tutorial, make your own Augmented Reality app.
https://scikit-surgerytutorial01.readthedocs.io/en/latest/?badge=latest
Other
8 stars 1 forks source link

Update requirements.txt to latest versions for consistency #13

Closed thompson318 closed 4 years ago

thompson318 commented 4 years ago

In GitLab by @MattClarkson on Nov 11, 2019, 19:39

requirements.txt is just looking a bit out of date.

thompson318 commented 4 years ago

In GitLab by @MattClarkson on Nov 11, 2019, 20:06

mentioned in commit eeac8f8b972cf76c1bdfbf500cada9ce423d8f1f

thompson318 commented 4 years ago

In GitLab by @MattClarkson on Nov 14, 2019, 08:55

As if the above commits, I get:

  master
(SNAPPYTutorial01-b) yisong-macbook:SNAPPYTutorial01 mattclarkson$ pip list
Package               Version            
--------------------- -------------------
certifi               2019.9.11          
numpy                 1.17.4             
opencv-contrib-python 4.1.1.26           
pip                   19.3.1             
PySide2               5.13.1             
scikit-surgerycore    0.1.7              
scikit-surgeryimage   0.6.2              
scikit-surgeryutils   0.6.2              
scikit-surgeryvtk     0.12.2             
setuptools            41.6.0.post20191030
shiboken2             5.13.1             
six                   1.13.0             
vtk                   8.1.2              
wheel                 0.33.6   
thompson318 commented 4 years ago

In GitLab by @MattClarkson on Nov 14, 2019, 09:03

@NinaMontanaBrown - are you thinking of going through this tutorial at all?

If so, it would be useful to test this again. With a clean environment, follow the instructions which now require pip install -r requirements-dev.txt which should then always pick up the correct versions.

So, now we just need to test that the existing tutorial does indeed work with those versions.... fingers crossed.

Note: You'd have to git checkout 13-update-dependencies to be on the right branch to pick up these code changes.

thompson318 commented 4 years ago

In GitLab by @MattClarkson on Nov 14, 2019, 09:04

mentioned in commit 47cdb0f67878e1b7be97f4cdb5c651e58e1729a0

thompson318 commented 4 years ago

In GitLab by @MattClarkson on Nov 14, 2019, 09:05

@NinaMontanaBrown - if you don't have time, I might be able to fit it in..... sometime, so don't worry.

thompson318 commented 4 years ago

In GitLab by @NinaMontanaBrown on Nov 14, 2019, 10:05

I was planning on going through this some time in the next few days. I'll keep you posted

thompson318 commented 4 years ago

In GitLab by @MattClarkson on Nov 18, 2019, 07:49

@StephenThompson - can you review. I was finding inconsistencies between requirements.txt and setup.py in downstream projects like scikit-surgeryvtk and scikit-surgeryutils. So, I made them consistent, merged, tagged and put the latest on PyPi. Then this project should run pip install -r requirements.txt to pick up the specified versions.

thompson318 commented 4 years ago

In GitLab by @StephenThompson on Nov 19, 2019, 12:31

The problem is the tutorial isn't really meant to be downloaded, you're meant to follow it on readthedocs, so they won't have requirements.txt. I think also, only libraries that are directly imported should be listed in requirements.txt. For the tutorial these are scikit-surgeryutils numpy opencv-contrib-python PySide2 The last three are dependencies of the first, so for ease of instruction I omitted them from the pip install instructions.

thompson318 commented 4 years ago

In GitLab by @MattClarkson on Nov 19, 2019, 12:41

ah. Fair point. I can revert if you like. What about specifying a version?

thompson318 commented 4 years ago

In GitLab by @StephenThompson on Nov 19, 2019, 12:51

I think it would be helpful is requirements.txt is correct so it will work if people do download it, so like you've done but with some removed, but I don't like the idea of specific versions, can't we stick with minimum versions, so; numpy>=1.17.4 PySide2>=5.13.1 opencv-contrib-python>=4.1.1.26 scikit-surgeryutils>=0.6.2

In terms of the instructions I think we should stick with just pip install scikit-surgeryutils

which should install the latest version anyway

thompson318 commented 4 years ago

In GitLab by @MattClarkson on Nov 19, 2019, 12:54

mentioned in commit 73550a832958b1b43c6d648991b1688281733e0c

thompson318 commented 4 years ago

In GitLab by @MattClarkson on Nov 19, 2019, 12:54

mentioned in commit 50a533c14a8b24394b200338ecc29eeec1249ebd

thompson318 commented 4 years ago

In GitLab by @MattClarkson on Nov 19, 2019, 14:02

mentioned in commit 5502b922ecf1c3163a8c48c0ed4db9c6a1023b4c

thompson318 commented 4 years ago

In GitLab by @MattClarkson on Nov 19, 2019, 14:03

done.

Also, I think I fixed an issue with a missing argument in the documentation. See merge request.

thompson318 commented 4 years ago

In GitLab by @MattClarkson on Nov 19, 2019, 14:05

mentioned in commit 57dca0f0b56308521f8032f6d025fe7c915ef6c6

thompson318 commented 4 years ago

In GitLab by @MattClarkson on Nov 19, 2019, 14:05

mentioned in merge request !1

thompson318 commented 4 years ago

In GitLab by @StephenThompson on Nov 19, 2019, 14:23

closed via merge request !1

thompson318 commented 4 years ago

In GitLab by @StephenThompson on Nov 19, 2019, 14:23

mentioned in commit 7063ec1534489fba6e56ba64fc2034159041481b