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: Remove support for PyQt4 #6961

Merged
merged 7 commits into from
May 21, 2018
Merged

Conversation

ccordoba12
Copy link
Member

Fixes #6889.

Also bump PyQt5 requirement to 5.5
Also:
- Add a new env var called USE_CONDA to clearly state what slots use
conda and what use pip.
- Add a new slot to test with Python 3.5 and conda.
@ccordoba12 ccordoba12 added this to the v3.3 milestone Apr 14, 2018
@pep8speaks
Copy link

pep8speaks commented Apr 14, 2018

Hello @ccordoba12! Thanks for updating the PR.

  • In the file setup.py, following are the PEP8 issues :

Line 244:7: E128 continuation line under-indented for visual indent

Line 749:9: E722 do not use bare except'
Line 765:9: E722 do not use bare except'

Line 115:80: E501 line too long (80 > 79 characters)

Comment last updated on May 21, 2018 at 21:04 Hours UTC

Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

Can we instead make the doc changes against the spyder-docs repo instead, since that's where they live now and will be backported to the main Spyder repo with each release? I'd be happy to make then myself right now if you'd like; I'm working a PR with a number of baseline updates and fixes like this for them of this nature. Otherwise, considering the changes already present there and that will be present shortly, that will be pulled back in to the main repo on the next release, it'll certainly be a lot of conflicts to resolve and in any case, I was under the impression that would be our "live" docs branch from now on, with this one just being included for now to maintain backward compatibility.

If that's not what you want us to do and we want to have a different system instead, then we can go ahead with that but please let me know so we're on the same page. Thanks!

bootstrap.py Outdated
except ImportError:
print("02. No PyQt5 or PyQt4 detected, using PySide if available "
"(deprecated)")
print("02. No PyQt5 detected (deprecated)")
Copy link
Member

Choose a reason for hiding this comment

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

What does (deprecated) refer to here? Before it referred to the deprecated pyside support but that was deleted, so since PyQt5 is (hopefully) not deprecated also we can presumably delete that parenthetical?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, I have to remove that deprecated.

* `PyQt5 <https://www.riverbankcomputing.com/software/pyqt/download5>`_ >=5.2 or
`PyQt4 <https://www.riverbankcomputing.com/software/pyqt/download>`_ >=4.6.0
(PyQt5 is recommended).
* `PyQt5 <https://www.riverbankcomputing.com/software/pyqt/download5>`_ >=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.

Doc changes; see top level comment.

Copy link
Member

Choose a reason for hiding this comment

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

@ccordoba12 Just FYI, if you don't get to it before I do my pass for baseline correctness updates (which will happen as soon as we resolve spyder-ide/spyder-docs#14 ) I can just do it as one part of that, as it is a close fit for the purpose and scope of the issue its resolving.

Spyder may also be used as a PyQt5 or PyQt4 extension library
(module 'spyder'). For example, the Python interactive shell widget
used in Spyder may be embedded in your own PyQt5 or PyQt4 application.
Spyder may also be used as a PyQt5 extension library (module
Copy link
Member

Choose a reason for hiding this comment

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

More doc changes.

setup.py Outdated
It also provides ready-to-use pure-Python widgets to your PyQt5 or
PyQt4 application: source code editor with syntax highlighting and
It also provides ready-to-use pure-Python widgets to your PyQt5
application: source code editor with syntax highlighting and
Copy link
Member

Choose a reason for hiding this comment

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

Should be "A source code editor"

@CAM-Gerlach
Copy link
Member

@ccordoba12 Left a comment with a few minor issues, mainly about the doc changes potentially going to the wrong repo.

@ccordoba12
Copy link
Member Author

Ok, I'll address your comments and also open a new PR in spyder-docs.

After this PR I'll make no further changes to our docs here. I'm also thinking that, since we bumped our version to 3.3, we can remove our docs from here, stop distributing them in our tarballs and instead display them only online, i.e. through our website.

@CAM-Gerlach
Copy link
Member

@ccordoba12 Okay, sure—whatever you think is best. Would you like me to open a PR to do that, or will you handle it? Presumably, along with deleting the files we'll also need to remove the relevant section in setup.py—anything else? Our CIs here aren't set up to build the docs or anything, right? Thanks.

@ccordoba12
Copy link
Member Author

Would you like me to open a PR to do that, or will you handle it?

Let me do it. I know all the things that have to be removed.

@CAM-Gerlach
Copy link
Member

CAM-Gerlach commented Apr 22, 2018

@ccordoba12 As part of this PR, would you also want to consider updating the minimum Qt/PyQt version to 5.6? In the thousands of bug reports from the past year, I don't know if I've seen a single user using 5.5, it is long unsupported not being a LTS like 5.6, it isn't compatible with the high dpi scaling Spyder uses, we don't test with it, and in some of these places the min Qt/PyQt version listed is even older, e.g. 5.2. There's also some other old code still around for compatibility with Qt <5.5 that this PR doesn't catch, like in mixins.py and codeeditor.py, though perhaps its out of scope.

@ccordoba12
Copy link
Member Author

As part of this PR, would you also want to consider updating the minimum Qt/PyQt version to 5.6?

It'd be more work for little gain. I think 5.5 is fine.

@ccordoba12 ccordoba12 dismissed CAM-Gerlach’s stale review May 21, 2018 21:07

Address review and open a PR in spyder-docs to remove mentions to PyQt4

@ccordoba12 ccordoba12 merged commit 2c6c90f into spyder-ide:3.x May 21, 2018
@ccordoba12 ccordoba12 deleted the remove-pyqt4 branch May 21, 2018 21:32
ccordoba12 added a commit that referenced this pull request May 21, 2018
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