baudren / NoteOrganiser

Scientific Note Taking
MIT License
13 stars 6 forks source link

[WIP] Qtpy #52

Closed baudren closed 8 years ago

baudren commented 9 years ago

I opened a PR to discuss the various problems with this. Currently, this is how my notebooks are displayed:

qtpy_glitch

The names are gone. I have the following trace:

 File "C:\Anaconda\lib\site-packages\noteorganiser-0.2.0_-py2.7.egg\noteorganiser\widgets.py", line 61, in paintEvent
    painter.drawText(event.rect(), elided)
TypeError: arguments did not match any overloaded call:
  QPainter.drawText(QPointF, QString): argument 1 has unexpected type 'QRect'
  QPainter.drawText(QRectF, int, QString): argument 1 has unexpected type 'QRect'
  QPainter.drawText(QRect, int, QString): argument 2 has unexpected type 'unicode'
  QPainter.drawText(QRectF, QString, QTextOption option=QTextOption()): argument 1 has unexpected type 'QRect'
  QPainter.drawText(QPoint, QString): argument 1 has unexpected type 'QRect'
  QPainter.drawText(int, int, int, int, int, QString): argument 1 has unexpected type 'QRect'
  QPainter.drawText(int, int, QString): argument 1 has unexpected type 'QRect'

Don't you have the same behaviour?

landscape-bot commented 9 years ago

Code Health Code quality remained the same when pulling e7f5e2a on baudren:qtpy into a7ba646 on baudren:devel.

egolus commented 9 years ago

No, I don't get any error and the names are visible

can you run the tests and look at the qt-api? Here and in travis it's pyside.

egolus commented 9 years ago

http://pythonhosted.org/pyqode.qt/pyqode.qt.html#module-pyqode.qt.QtGui this is the 'API-reference' for pyqode.qt where qtpy is based on.

All PyQt4/PySide gui classes are exposed but when you use PyQt5, those classes are not available. Therefore, you should treat/use this package as if it was PyQt5.QtWidgets module.

This sounds uncomfortable. You use a library to give you a uniform api and it's not uniform?

baudren commented 9 years ago

More importantly on this page: QTest is not supported?

baudren commented 9 years ago

after testing more thoroughly, I am seeing that if two bindings are installed on the machine, qtpy is confused and mixes them (tests are failing). This is really not an acceptable behaviour for a shim, which is supposed to ease the problems, and allow to use one or the other. I would suggest to drop this idea - unfortunately, because in itself, it is a nice one!

egolus commented 9 years ago

are you sure it mixes them? I think, our problem is, that we use the pyside syntax and objects. If we would use clean PyQt syntax, it would work with both bindings.

I was trying to test this but it's not that easy to install PyQt on windows as there is no pip-package

landscape-bot commented 9 years ago

Code Health Repository health decreased by 30% when pulling 61fa65c on qtpy into a7ba646 on devel.

landscape-bot commented 9 years ago

Code Health Repository health decreased by 37% when pulling cebac7a on qtpy into a7ba646 on devel.

landscape-bot commented 9 years ago

Code Health Repository health decreased by 0.30% when pulling 3336428 on qtpy into f6f26e8 on devel.

egolus commented 9 years ago

sorry for the forced push. I decided to rewrite this.

The current status:

egolus commented 9 years ago

new things I found out:

what still doesn't work is the webview when the PyQt API is used:


If the tests for py3 fail when having both pyside and pyqt installed, it's because of the different order when looking for the QT_API: pytest finds pyside first, qtpy finds PyQt first. So qtpy loads the PyQt API but pytest assumes the pyside API is loaded. If you set either of the environment variables, the tests should work

landscape-bot commented 9 years ago

Code Health Repository health decreased by 0.30% when pulling 47b1056 on qtpy into f6f26e8 on devel.

landscape-bot commented 9 years ago

Code Health Repository health decreased by 0.30% when pulling 47b1056 on qtpy into f6f26e8 on devel.

baudren commented 9 years ago

at least now, the text appears in the notebook icons, so, there's that. Good work so far.

The problem of qtest selecting the framework in a different order seems tough... on a multi-bindings platform, the tests will never work, and I am reluctant to have that. Because setting the bindings in the code means that someone without this binding will get stranded. That is not robust.

egolus commented 9 years ago

I feel like we have to set the API before running the tests. The tests are mainly for us as developers, not for a normal user that just wants to use the software. The problem with QWebView shows that we even have to test for both pyside and PyQt. So make test should do the following:

what I still don't like is having the pyside specific bindings when using qtpy with pyside. I'd like to get at least a warning. But if you look at the qtpy code, you see it just does *-Imports, so there will be no warnings.

So I allways have to switch the QT API to check if I'm doing something dumb (using a pyside specific binding)

baudren commented 9 years ago

Ok. Let's add an argument, then, to set the API from the command line. Why do you exclude test-2 PyQt? Is it not available?

egolus commented 9 years ago

I should have written PyQt5 and that is only available for python3, right?

baudren commented 9 years ago

It is actually compatible with Python 2, but is not packaged like this for Ubuntu (at least). So, yes, to ease the burden, testing it only on Python 3 should be enough.

egolus commented 9 years ago

hmm ok, I'll check. I installed it on windows and it's only installed for python3 ..

egolus commented 9 years ago

on the not working QWebView: If I change this line to self.web.load(QtCore.QUrl.fromLocalFile(page)) the QWebView is showing the page on PyQt5, but there is an Error when running the tests: (running the tests with pyside as QT API)

______________________ ERROR at teardown of test_preview ______________________

    def fin():
        """Tear down parent class"""
>       shutil.rmtree(temp_folder_path)

noteorganiser\tests\custom_fixtures.py:48:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
c:\PROGRA~1\python27\lib\shutil.py:247: in rmtree
    rmtree(fullname, ignore_errors, onerror)
c:\PROGRA~1\python27\lib\shutil.py:252: in rmtree
    onerror(os.remove, fullname, sys.exc_info())
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

path = 'C:\\Users\\Benutzer2\\.noteorganiser\\.test_2015-07-06\\.website'
ignore_errors = False, onerror = <function onerror at 0x00000000044E2DD8>

    def rmtree(path, ignore_errors=False, onerror=None):
        """Recursively delete a directory tree.

        If ignore_errors is set, errors are ignored; otherwise, if onerror
        is set, it is called to handle the error with arguments (func,
        path, exc_info) where func is os.listdir, os.remove, or os.rmdir;
        path is the argument to that function that caused it to fail; and
        exc_info is a tuple returned by sys.exc_info().  If ignore_errors
        is false and onerror is None, an exception is raised.

        """
        if ignore_errors:
            def onerror(*args):
                pass
        elif onerror is None:
            def onerror(*args):
                raise
        try:
            if os.path.islink(path):
                # symlinks to directories are forbidden, see bug #1669
                raise OSError("Cannot call rmtree on a symbolic link")
        except OSError:
            onerror(os.path.islink, path, sys.exc_info())
            # can't continue even if onerror hook returns
            return
        names = []
        try:
            names = os.listdir(path)
        except os.error, err:
            onerror(os.listdir, path, sys.exc_info())
        for name in names:
            fullname = os.path.join(path, name)
            try:
                mode = os.lstat(fullname).st_mode
            except os.error:
                mode = 0
            if stat.S_ISDIR(mode):
                rmtree(fullname, ignore_errors, onerror)
            else:
                try:
>                   os.remove(fullname)
E                   WindowsError: [Error 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\Benutzer2\\.noteorganiser\\.test_2015-07-06\\.website\\example.html'

c:\PROGRA~1\python27\lib\shutil.py:250: WindowsError

It looks like the local URL keeps the file locked? Or maybe it's just a timing issue?

the other error with the missing method QWebView.textSizeMultiplier is even weirder. I don't find any reason why it doesn't exist.

baudren commented 8 years ago

Let's close this for the moment, and merge #73. Maybe one day we will make it work, but so far, this problem with pyqt5 is a show-stopper.