Solves issues #335 and #336

mxcube / mxcubeqt

Qt Front-end of MXCuBE

http://mxcube.github.io/mxcube/

GNU Lesser General Public License v3.0

14 stars 34 forks source link

Solves issues #335 and #336 #337

Closed IvarsKarpics closed 5 years ago

IvarsKarpics commented 5 years ago

Added python3 support for travis (new scripts to install pyqt4 and pyqt5)
Fixed QtImport for python3 (removed star import)

Solves issues #335 and #336

jordiandreu commented 5 years ago

Hi @IvarsKarpics,

I have tested the latest mxcube release 2.3.0.1 (mockup gui) using:

python=3.7.3
qt=5.6.2
pyqt=5.6.0
matplotlib 3.0.3

...and I have a couple of comments related to the requirement files for python3:

I think that the enum34 module is not required for python>=3.4 (which python version should be recommended for MXCuBE?).
- I got an exception regarding the loading of the redis module, which is mandatory to load the RedisClient. This should not be mandatory for the Qt version, right?

jordiandreu commented 5 years ago

Hi @IvarsKarpics,

I have also tested the latest mxcube release 2.3.0.1 (mockup gui) using:

python=2.7.13 qt=4.8.7 pyqt=4.11.4 matplotlib 2.0.0

The requirements looks fine, except for the redis issue again.

IvarsKarpics commented 5 years ago

Hi @jordiandreu , Thanks for the valuable feedback! Redis hwobj and redis library is optional for the qt version. I will remove the redis entry from the example configuration.

IvarsKarpics commented 5 years ago

@rhfogh , @jordiandreu Is it fine to merge the PR?

jordiandreu commented 5 years ago

@IvarsKarpics , I would recommend to specify number version for dependencies, event not mandatory, it is much clear to create environments for debugging and reproduce errors.

Does pip fail to install enum34 if Python >=3.4? (it fails in conda environment creation) In this case, I would remove the enum34 from requirements if we work in Python >=3.4.

rhfogh commented 5 years ago

My opinion is not worth much, not being on top of the testing, but FWIW I am fine with merging - hence my previous approval ;-)

On 16/04/2019 08:27, Ivars Karpics wrote:

@rhfogh https://github.com/rhfogh , @jordiandreu https://github.com/jordiandreu Is it fine to merge the PR?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mxcube/mxcube/pull/337#issuecomment-483544049, or mute the thread https://github.com/notifications/unsubscribe-auth/AJXYEFTWBj1kVcxcL3s1y7v1N7_mDxuwks5vhXtegaJpZM4ciOhx.

-- Rasmus Fogh Cambridge, UK.

IvarsKarpics commented 5 years ago

Ok @jordiandreu , I removed the enum34 from the python3 requirements.

IvarsKarpics commented 5 years ago

Can someone merge the PR? Or is there something else to do?

IvarsKarpics commented 5 years ago

Thanks @rhfogh !