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

Ignore case for QT_API env variable in qtpy submodules #33

Merged
merged 4 commits into from
Jun 11, 2016

Conversation

Nodd
Copy link
Contributor

@Nodd Nodd commented Jun 10, 2016

This is easier on the users, and the tests are currently failing because of this problem.

I got rid of the os.environ[QT_API] lookups, because it makes qtpy rely too much on an external global state, while the environent is already pared in __init__.py. This makes the import thread safe, for example.

With this I think that spyder-ide/spyder#3210 won't fail anymore.

import os

from qtpy import QT_API
from qtpy import API
from qtpy import PYQT5_API
from qtpy import PYQT4_API
from qtpy import PYSIDE_API
from qtpy import PythonQtError
Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW, why are those imports split into multiple lines ?

@ccordoba12 ccordoba12 added this to the v1.1 milestone Jun 11, 2016
@ccordoba12
Copy link
Member

I like this one. Please unify PYQT5, etc imports in a single line and fix conflicts to merge it :-)

@Nodd
Copy link
Contributor Author

Nodd commented Jun 11, 2016

It should be good, waiting for tests.

@ccordoba12 ccordoba12 changed the title Ignore case for QT_API env variable Ignore case for QT_API env variable in qtpy submodules Jun 11, 2016
@Nodd Nodd mentioned this pull request Jun 11, 2016
@ccordoba12 ccordoba12 merged commit 41fc86b into spyder-ide:master Jun 11, 2016
@Nodd Nodd deleted the ignore_case branch June 11, 2016 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants