Deprecation Warning for Enum Access

[adam-grant-hendry]

Per this comment here:

starting with sip v4.19.9 (around PyQt 5.11) all enums are now scoped, and starting in PyQt6, the backwards compatibility allowing enums to be accessed in their top level namespace has been removed

This is confirmed by the PyQt5 Gotcha's docs.

Since end users may be accessing different versions of PyQt/PySide through qtpy, would it be possible to add a DeprecationWarning for the corresponding versions? (most likely this could be determined at runtime via QT_API)

e.g. I noticed a "Type[QEvent]" has no attribute "Gesture" mypy error in legacy code where Gesture was accessed like so:

and the fix was instead to use QtCore.QEvent.Type.Gesture.

Perhaps something like:

DeprecationWarning: Enums are scoped beginning with PyQt 5.11 (and its corresponding PySide 2 version). Access to enums through top-level namespaces has been deprecated beginning in PyQt 6.

Use the full path to the enumeration instead. For example:

QtCore.QEvent.Type.Gesture

instead of 

QtCore.QEvent.Gesture

[CAM-Gerlach]

Hey, thanks for the detailed proposal.

With PR #271 , QtPy >=2.0.0 (first version supporting Qt6) supports unscoped enum access even on PyQt versions that don't support it natively, for consistency and abstraction between bindings (since PySide handles this differently). Indeed, our tests contain this assert testing that an example QtCore.QEvent enum works correctly across all supported Qt and binding versions:

     assert QtCore.QEvent.ActionAdded == QtCore.QEvent.Type.ActionAdded

Therefore, unless I'm mistaken about what you're asking, I'm not sure there's a need for a DeprecationWarning on QtPy's side.

@dalthviz , any thoughts here?

[adam-grant-hendry]

@CAM-Gerlach Thank you for the pointing me to PR #271 .

I noticed here that enums were promoted for PyQt6, but not PySide6. Is there a reason for that?

I'm wondering because I'm using these PySide6 stubs and the (relevant part of the) implementation for QEvent is (save for lines I modified, commented below):

from enum import IntFlag  # was "from enum import Enum"
from typing import overload

import PySide6.QtCore
import shiboken6 as Shiboken

class QEvent(Shiboken.Object):  # was "QEvent(object)"
    None_: QEvent.Type = ...
    Timer: QEvent.Type = ...
    ...
    class Type(IntFlag):  # was "Type(Enum)"
        None_: QEvent.Type = ...
        Timer: QEvent.Type = ...
        ... # same enums as in top-level

    @overload
    def __init__(self, arg__1: PySide6.QtCore.QEvent) -> None:
        ...
    @overload
    def __init__(self, arg__1: PySide6.QtCore.QEvent.Type) -> None:
        ...
    
    ... # remaining methods overloaded    

but I am still receiving a mypy linting error.

[CAM-Gerlach]

I'm not sure, but @dalthviz and the various folks who discussed it on #233 likely have a better idea...

Also, FYI, just to make sure you're aware of our qtpy mpy-args CLI added in #337 that allow you to tell mypy which API it should assume at type-check-time?

[adam-grant-hendry]

Also, FYI, just to make sure you're aware of our qtpy mpy-args CLI added in #337 that allow you to tell mypy which API it should assume at type-check-time?

+1 on that. No! I wasn't aware! I'm going to look right now.

[adam-grant-hendry]

@CAM-Gerlach After reading the docs, I'm still a little confused. How would I use the --always-true/--always-false options? I use powershell on Windows.

Specifically though, I'd like it to work in VSCode. I have a Python 3.8.10 virtual environment created through venv in a folder .venv in my project folder.

I've tried this in my workspace settings.json:

{
    "python.linting.mypyEnabled": true,
    "python.linting.mypyPath": "${workspaceFolder}/.venv/Scripts/pylint.exe",
    "python.linting.mypyArgs": [
        "--config-file",
        "mypy.ini",
        "--always-true=PYSIDE6",
        "--always-false=PYQT5,PYSIDE2,PYQT6"
    ],
    "mypy.dmypyExecutable": "${workspaceFolder}/.venv/Scripts/dmypy.exe"
}

and the mypy.ini just has:

[mypy]
disallow_untyped_defs = True

[adam-grant-hendry]

Can the above also be given/passed to pylint, or does it only work with mypy?

[adam-grant-hendry]

I can confirm (using the Python OUTPUT tab) when the arguments are passed separately (not comma-delimited), this works for mypy:

> .\.venv\Scripts\python.exe ~\.vscode\extensions\ms-python.python-2022.8.1\pythonFiles\linter.py -p mypy ./.venv/Scripts/mypy.exe --config-file mypy.ini --always-true=PYSIDE6 --always-false=PYQT5,PYSIDE2,PYQT6 .\pyvistaqt\window.py
cwd: .
##########Linting Output - mypy##########

pyvistaqt\window.py:33: error: "Type[QEvent]" has no attribute "Gesture"
Found 1 error in 1 file (checked 1 source file)

as opposed to

> .\.venv\Scripts\python.exe ~\.vscode\extensions\ms-python.python-2022.8.1\pythonFiles\linter.py -p mypy ./.venv/Scripts/mypy.exe --config-file mypy.ini --always-true=PYSIDE6 --always-false=PYQT5 --always-false=PYSIDE2 --always-false=PYQT6 .\pyvistaqt\window.py
cwd: .
##########Linting Output - mypy##########

pyvistaqt\window.py:34: error: "Signal" has no attribute "emit"
pyvistaqt\window.py:40: error: "Signal" has no attribute "emit"
Found 2 errors in 1 file (checked 1 source file)

and I know how to fix the stubs to solve the error "Signal" has no attribute "emit".

However, I have yet to figure out a way to get this to work for pylint. It defaults to using PySide2.

[adam-grant-hendry]

The above is a little strange considering that the mypy docs state always_true and always_false are each a

comma-separated list of strings

[adam-grant-hendry]

I figured out the last piece of the puzzle here. I mispoke: I wasn't needing an argument for pylint, I was needing it for pyright. By default, the Microsoft Python language server Pylance uses pyright under the hood. I can use its defineConstant argument as define here to get Pylance to honor the same values for qtpy.

In a pyproject.toml, this is

[tool.pyright]
defineConstant = [
  { PYSIDE6 = true },
  { PYQT5 = false },
  { PYSIDE2 = false },
  { PYQT6 = false }
]

One can also define a pyrightconfig.json instead, as the docs explain.

Finally, it was important for me to ONLY have the Microsoft Python extension installed in VSCode, and not this 3rd party mypy extension. The problem with this extension is that you cannot pass extra arguments to it beyond the path to the config file, and I needed to not specify the --always-true arguments directly in my mypy.ini, but rather in my workspace settings.json:

"python.linting.mypyArgs": [
        "--config-file",
        "mypy.ini",
        "--always-true=PYSIDE6",
        "--always-false=PYQT5",
        "--always-false=PYSIDE2",
        "--always-false=PYQT6",
]

[CAM-Gerlach]

Great, sorry it was so much trouble and thanks for all the info—might certainly be helpful for someone else in the future.

Are there any changes you would recommend to QtPy based on this, either in the code or in the docs? If so, we'd welcome a PR, or I can try to give it a shot if you can advise me accordingly. Thanks!

[adam-grant-hendry]

@CAM-Gerlach I'd still like to know why enums weren't promoted for PySide6. If you could have someone answer that for me, that would be great!

[CAM-Gerlach]

Sure; @dalthviz ?

[adam-grant-hendry]

Sure; @dalthviz ?

Thank you! The code works (obviously), so I'm guessing perhaps it's something to do with the difference between the how sip generates bindings vs shiboken, but I'm not sure (I'd like to learn more).

[dalthviz]

Hi @adam-grant-hendry, thank you for the feedback and sharing with us the things you did to configure your setup with qtpy! Also thank you @CAM-Gerlach for giving guidance to @adam-grant-hendry to find the correct setup 👍

Regarding the reason behind why enums where not promoted for PySide6 is basically because it was not necessary. As you guessed/checked, there is a difference between what sip and shiboken enable regarding the unscoped access of enums. For the moment PyQt6 is the only binding which fully removed the unscoped access to enums. Following that, and since we try follow the modules layout available with Qt5, we decided to keep the access here by promoting the enums when using PyQt6.

For more details you can check some comments like #233 (comment) where we discussed the unscoped vs scoped enums access.

Also, you can check the logic we are using to allow the unscoped enums access here: https://github.com/dalthviz/qtpy/blob/b02425b2f884ccefaca25d904f5d61bae14756a4/qtpy/enums_compat.py#L19-L37 (basically looping from a given module all the sip.wrappertype classes available and then adding the attributes/values available on their enums).

If you have any other question or comment let us know!

[CAM-Gerlach]

I guess the one potential concern I'd have would be if DeprecationWarnings are being used for use of unscoped enums on any of our supported bindings that still allow them natively, which would be somewhat inconsistent and confusing to users given the fact that they aren't present at all on PyQt6 yet don't warn. @adam-grant-hendry @dalthviz have you noticed any DeprecationWarnings about this?

[adam-grant-hendry]

@CAM-Gerlach @dalthviz Thank you both SO MUCH!

There was a lot of discussion here and I really appreciate all the help and explanation. This was a wonderful experience! Top marks!

As a follow up to @CAM-Gerlach 's question about suggestions for the docs, only two things come to mind:

1. If you could add how to configure pyright and to add to settings.json as I showed here, I think that would help future developers.

2. I discovered the odd behavior I was experiencing was due to pre-commit, not Pylance or mypy directly. This issue post helped me understand the reasons:

   • pre-commit uses a mirror repo for mypy that has 2 issues:

     (a) pre-commit is run with files as positional arguments, which is why I suspect I see different results depending on whether I comma-separate the --always-false arguments instead of put them on separate lines

     (b) the mypy in pre-commit runs in an isolated environment, so it won't have access to your dependencies (i.e. stub file packages) unless you specify them as dependencies (using additional_dependencies) or if you add the language: system argument to the hook. From the the pre-commit docs:

System hooks provide a way to write hooks for system-level executables which don't have a supported language above (or have special environment requirements that don't allow them to run in isolation such as pylint).

This hook type will not be given a virtual environment to work with – if it needs additional dependencies the consumer must install them manually.

This issue post helped me understand "system" basically means rely on whatever state the user's machine is in, so you'll have to configure settings appropriately in CI vs on your system

[CAM-Gerlach]

If you could add how to configure pyright and to add to settings.json #352 (comment), I think that would help future developers.

Maybe we could just include a short note with a link to your aforementioned comment here, rather than pasting the whole thing there?

• (b) the mypy in pre-commit runs in an isolated environment, so it won't have access to your dependencies (i.e. stub file packages) unless you specify them as dependencies (using additional_dependencies) or if you add the language: system argument to the hook.

Typically, unless the project has relatively few or no third party deps, I typically run mypy (as well as Pylint and Pyanalyze) using system in a local hook, where I also use mypy . as the invocation and pass_filenames: false, which (incidentally) avoids issue (a) as well. I typically also have a check-env-activated custom hook that runs before it and checks if an appropriate Python environment (i.e. containing the required dependencies for the package itself and the hooks) is activated, as a quick and obvious check in case it isn't. Here's what the config looks like:

  - id: check-env-activated
    name: Check that a suitable Python env is activated
    entry: 'python tools/check_env_activated.py'
    language: system
    types: [python]
    pass_filenames: false
  - id: mypy
    name: Lint Python with MyPy
    entry: 'mypy .'
    language: system
    types: [python]
    pass_filenames: false

and here's the check_env_activated script.

[adam-grant-hendry]

have you noticed any DeprecationWarnings about this?

I have not, personally (thankfully). I was also concerned about the discrepancy across bindings and versions. As long as qtpy continues to support the same syntax (which I believe is the PyQt5 < 5.11 syntax, I don't think any warnings are required.

They would only be necessary in the individual PyQt/PySide bindings themselves (where syntax has changed/deprecated), and thus would have to be handled by those packages.

[adam-grant-hendry]

Maybe we could just include a short note with a link to your aforementioned comment here, rather than pasting the whole thing there?

Completely fine by me. However you would like to do it. You don't even have to add anything if you don't want to 😃 .

And thank you for the other advice about pre-commit! 👍

[adam-grant-hendry]

@CAM-Gerlach @dalthviz Whenever you're ready, feel free to close this issue. Thank you again!

[CAM-Gerlach]

Thanks for all your help and detailed comments! I opened PR #353 which adds such a note, and will close this issue when merged.