Cannot open preferences dialog because of missing PYQT5 constant

[drammock]

What steps will reproduce the problem?

1. Start spyder
2. click on Tools > Preferences

What is the expected output?
preferences dialog should open

What do you see instead?
internal console pops up:

Traceback (most recent call last):
  File "/home/drmccloy/Builds/spyder/spyderlib/app/spyder.py", line 2649, in edit_preferences
    widget.initialize()
  File "/home/drmccloy/Builds/spyder/spyderlib/plugins/configdialog.py", line 71, in initialize
    self.setup_page()
  File "/home/drmccloy/Builds/spyder/spyderlib/plugins/shortcuts.py", line 794, in setup_page
    if PYQT5:
NameError: global name 'PYQT5' is not defined

Please provide any additional information below

Versions and main components

from bootstrap output:
03. Imported Spyder 3.0.0dev
[Python 2.7.6 64bits, Qt 4.8.6, PyQt4 (API v2) 4.10.4 on Linux]

Dependencies

jedi >=0.8.1 : 0.9.0 (OK)
matplotlib >=1.0: 1.5.1+1643.g0423430 (OK)
nbconvert >=4.0 : 4.2.0 (OK)
numpy >=1.7 : 1.12.0.dev0+2af06c8 (OK)
pandas >=0.13.1 : 0.18.0+104.g1c8816f (OK)
pep8 >=0.6 : 1.7.0 (OK)
psutil >=0.3 : 1.2.1 (OK)
pyflakes >=0.5.0: 1.1.0 (OK)
pygments >=1.6 : 2.1.3 (OK)
pylint >=0.25 : 1.5.5 (OK)
qtconsole >=4.0 : 4.2.1 (OK)
rope >=0.9.4 : 0.10.3 (OK)
sphinx >=0.6.6 : 1.5a0 (OK)
sympy >=0.7.3 : None (NOK)

[goanpeca]

@ccordoba12 related to qtpy changes?

[goanpeca]

Also we should add qtoy to the dependencies, so it gets listed here when reporting issues

[drammock]

[~] $ pip list | grep QtPy
QtPy (1.0.1)

[goanpeca]

@drammock looking into it, thanks or reporting

[goanpeca]

@drammock can you check again now, just made a change in Master

[drammock]

fixed, yes. Though I'm a bit surprised that you closed the issue before I tested it.

[ccordoba12]

I'm sure @goanpeca tested it locally :-) It was a very simple fix anyway, so I approve his action.

[goanpeca]

@ccordoba12 this could be avoided if we enforced merging with master all PR's before final merging.

Do you think we can start doing this?

[drammock]

@goanpeca I strongly endorse the model of never pushing directly to master; only merging into master from pull requests that have been reviewed/approved by a maintainer who is a different person from the patch author. Besides having two pairs of eyes on every change, it also triggers the CI services to test the PR before it is merged. If your tests are thorough, it can catch a lot of bugs before users are affected.

[goanpeca]

@drammock, I was asking @ccordoba12 about what you ended up answering... and now on your opinion let me explain:

All the devs have agreed that we use PRs and review process is very thorough, in some rare circumstances (left to the good judgement of the developer) a 1 liner might be considered harmless enough for for a direct commit to master, which was the case. As @ccordoba12 pointed out, I checked locally that it worked and asked for your confirmation as a formality.

My comment was not referring to committing to master which did not cause this problem at all. The problem was that a PR that was merged and made a big migration was merged before another PR that made a fix to some problem with Qt5. If the later PR had been merged with master as a final step before merging, the automatic tests would have probably caught the problem, and hence why I am suggesting that all PRs from now on should be merged with master to be sure that cases like this can be avoided.

[drammock]

ah, I see, I misunderstood the question.

[goanpeca]

No problem :-), ideas are always welcome, and we are always grateful to users 👍 that test the application with Master (not many as you can imagine)

[ccordoba12]

@goanpeca the problem is not merging with master, it's that we are not testing the Preferences dialog. That's why this went unnoticed.

I don't know how to do that without running the entire application. If you could find a way, that'd be great!