Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR: Use QRegularExpression only for Qt 5.5+ #3994

Merged
merged 3 commits into from
Jan 20, 2017

Conversation

rlaverde
Copy link
Member

Fixes #3989

@rlaverde rlaverde added this to the v3.1.1 milestone Jan 18, 2017
@rlaverde rlaverde self-assigned this Jan 18, 2017
@rlaverde rlaverde requested a review from ccordoba12 January 18, 2017 16:43

if PYQT5:
PYQT_VERSION = float(PYQT_VERSION)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe its cleaner to define

PYQT55_VERSION = float(PYQT_VERSION) > 5.5

And use that in the conditions below?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm no, It's given an error due versions like 4.11.4

I'll implement:
MINOR_VERSION = int(PYQT_VERSION.split('.')[1])

@rlaverde rlaverde force-pushed the qregularexpression-qt55 branch from a1efb7a to 8459149 Compare January 18, 2017 17:20
@goanpeca
Copy link
Member

Well.. sure, I meant using a single global thing, not how you were comparing versions (which, yes is probably wrong)

No need

You can compare tuples also

a = (5, 5)
b = (5, 5, 1)

a > b
b < a

But I guess what you did now works also

@rlaverde rlaverde force-pushed the qregularexpression-qt55 branch from 8459149 to c77dbd1 Compare January 18, 2017 17:48
@rlaverde
Copy link
Member Author

@goanpeca I followed your both advices, I think this is ready :)


if PYQT5:
PYQT55_VERSION = tuple(int(x) for x in PYQT_VERSION.split('.')) >= (5, 5)
Copy link
Member

@ccordoba12 ccordoba12 Jan 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rlaverde, although Qt and PyQt versions usually match (e.g. Qt 5.5 and PyQt 5.5 are installed at the same time), that's not always the case. For example, Ubuntu 16.10 comes with Qt 5.6 and PyQt 5.7.

So please use QT_VERSION instead of PYQT_VERSION here. We define QT_VERSION here:

https://github.com/spyder-ide/qtpy/blob/master/qtpy/__init__.py#L87

@ccordoba12 ccordoba12 changed the title PR: Use QRegularExpression only if PYQT_VERSION >= 5.5 PR: Use QRegularExpression only for Qt 5.5+ Jan 19, 2017

if PYQT5:
QT55_VERSION = tuple(int(x) for x in QT_VERSION.split('.')) >= (5, 5)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use check_version from utils/programs.py to do the comparison here

@ccordoba12
Copy link
Member

Thanks @rlaverde!

@ccordoba12 ccordoba12 merged commit f60842d into spyder-ide:3.1.x Jan 20, 2017
ccordoba12 added a commit that referenced this pull request Jan 20, 2017
@rlaverde rlaverde deleted the qregularexpression-qt55 branch January 20, 2017 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants