Fix WindowType and WindowFlags integer operations

[bluebird75]

Before the fix, the test would report:

windowflags.py:10: error: Incompatible types in assignment (expression has type "int", variable has type "WindowType")
windowflags.py:18: error: Incompatible types in assignment (expression has type "int", variable has type "WindowFlags")
windowflags.py:19: error: Incompatible types in assignment (expression has type "int", variable has type "WindowFlags")
windowflags.py:20: error: Incompatible types in assignment (expression has type "int", variable has type "WindowFlags")
windowflags.py:21: error: Incompatible types in assignment (expression has type "int", variable has type "WindowFlags")
windowflags.py:22: error: Incompatible types in assignment (expression has type "int", variable has type "WindowFlags")
windowflags.py:27: error: Unsupported operand types for + ("WindowFlags" and "WindowType")
windowflags.py:27: error: Incompatible types in assignment (expression has type "int", variable has type "WindowFlags")
windowflags.py:28: error: Unsupported operand types for - ("WindowFlags" and "WindowType")
windowflags.py:28: error: Incompatible types in assignment (expression has type "int", variable has type "WindowFlags")
windowflags.py:29: error: Unsupported operand types for | ("WindowFlags" and "WindowType")
windowflags.py:29: error: Incompatible types in assignment (expression has type "int", variable has type "WindowFlags")
windowflags.py:30: error: Unsupported operand types for & ("WindowFlags" and "WindowType")
windowflags.py:30: error: Incompatible types in assignment (expression has type "int", variable has type "WindowFlags")
windowflags.py:31: error: Unsupported operand types for ^ ("WindowFlags" and "WindowType")
windowflags.py:31: error: Incompatible types in assignment (expression has type "int", variable has type "WindowFlags")
windowflags.py:34: error: Incompatible types in assignment (expression has type "int", variable has type "WindowFlags")
windowflags.py:34: error: Unsupported operand types for + ("WindowType" and "WindowFlags")
windowflags.py:35: error: Incompatible types in assignment (expression has type "int", variable has type "WindowFlags")
windowflags.py:35: error: Unsupported operand types for - ("WindowType" and "WindowFlags")
windowflags.py:36: error: Incompatible types in assignment (expression has type "int", variable has type "WindowFlags")
windowflags.py:36: error: Unsupported operand types for | ("WindowType" and "WindowFlags")
windowflags.py:37: error: Incompatible types in assignment (expression has type "int", variable has type "WindowFlags")
windowflags.py:37: error: Unsupported operand types for & ("WindowType" and "WindowFlags")
windowflags.py:38: error: Incompatible types in assignment (expression has type "int", variable has type "WindowFlags")
windowflags.py:38: error: Unsupported operand types for ^ ("WindowType" and "WindowFlags")
windowflags.py:41: error: Unsupported left operand type for + ("WindowFlags")
windowflags.py:42: error: Unsupported left operand type for - ("WindowFlags")
windowflags.py:43: error: Unsupported left operand type for | ("WindowFlags")
windowflags.py:44: error: Unsupported left operand type for & ("WindowFlags")
windowflags.py:45: error: Unsupported left operand type for ^ ("WindowFlags")

[bluebird75]

fixes #142

[BryceBeagle]

Looks like these changes cause the tests to fail in CI.

Also can you add an entry to the CHANGELOG.md? We should probably document PR workflow somewhere...

[bluebird75]

This was more difficult than I expected, I had to cover all cases to make sure it works correctly. Let's see what CI says.

[bluebird75]

I just re-read carefully the Qt documentation about QFlags ( https://doc.qt.io/qt-5/qflags.html#details ) . The following points are interesting :

If you try to pass a value from another enum or just a plain integer other than 0, the compiler will report an error. If you need to cast integer values to flags in a untyped fashion, you can use the explicit QFlags constructor as cast operator.

Then makes me think that combinations of QFlags with int is discouraged and maybe, I should raise a TypeError there just like in C++. Looking at the operator implemented in QFlags for int, I see :

• operator & and &= both supports int (like I did)
• operator | |= ^ ^= do not support int at all (unlike what I did)
• no addition or substraction is definied (like I did)

So, we could be theorically faithful to QFlags by removing int support in OR and XOR operators, or we could just reflect the PyQt reality that it works fine also it is discouraged. I lean toward the first case (limiting int support) because it provides more typesafety in using those QFlags.

[BryceBeagle]

So, we could be theorically faithful to QFlags by removing int support in OR and XOR operators, or we could just reflect the PyQt reality that it works fine also it is discouraged. I lean toward the first case (limiting int support) because it provides more typesafety in using those QFlags.

I think so far we've been maintaining these stubs to be true to the PyQt implementation so far, so I'd be a bit hesitant to start making them opinionated. Maybe we could reach out to Phil on the mailing list to see if he'd want to make PyQt itself better reflect Qt's design? He's already been changing a lot of things about the Enums/Flags in PyQt6; maybe he'd be open to making this change too.

[bluebird75]

I'll try to reach to Phil Thompson but there a decision to be made here anyway :-) . I like it how it is now as far as I am concerned.

[altendky]

@bluebird75, I see you left a couple comments saying things need to change and then one saying you like it how it is. CI is failing so something needs to change somewhere. I haven't yet looked to figure out where. We certainly do have plenty of ignores strewn about for places where accurately representing what PyQt does results in Mypy complaining that we are doing bad things. You can see the # type: ignore[misc] over in https://github.com/python-qt-tools/PyQt5-stubs/pull/109/files, for example. I know there are a lot of ignores specifically for Liskov violations. As mentioned, so far we have focused on accuracy over driving users towards a certain 'style'.

Let us know if you want to make some changes or if you'd like us to jump in and help figure this out.

[BryceBeagle]

@BryceBeagle, had you gone over this in any detail? Definitely no need for us both to do so.

No, I haven't had a chance to look into this in detail. I've just been making some high-level comments/observations. :+1:

[bluebird75]

finally, ready to be merged !

[altendky]

Are you still working on this or would you like me to see if I can help with the failures?

[bluebird75]

I would like to get this one done, because on the principle, it's a much more appropriate way to describe a QFlag based class. Assuming we get it to work here, all I need to do on other qflag based classes is change the inheritance.

I'll give it a short attempt on fixing it and ask for help if I am unsuccessful.

[bluebird75]

The CI fails on the mypy check but it passes fine on my computer :

d:\work\pyqt5-stubs\PyQt5-stubs>mypy --show-error-codes  PyQt5-stubs
Success: no issues found in 46 source files

d:\work\pyqt5-stubs\PyQt5-stubs>mypy --version
mypy 0.812

Which brings two questions :

1. which version of mypy is good enough for pyqt5-stubs verification ? I see that the requirements has pinned down a very specific commit.

2. the failure is about returning an invariant type where mypy would expect a covariant (as argument) or a contra variant (as a return value). While this is a fine approach in general, in this specific case, returning an invariant is actually correct because the API is really set in stone. This is probably why this error has been removed with later versions of mypy.

I can probably fix it by using co/contra variants but I would like to be settled on the mypy version first.

[bluebird75]

OK. One clear problem is that I did not setup my local requirements per requirements\develop.in . If I do that, I am sure that I will use the expected version of mypy.

Still, it would be a good idea to have a CI run with the latest mypy to make sure there are no differences for users using the most recent version.

[bluebird75]

I tried to setup the equivalent of the CI on my computer using the classical :

pip install -r requirements\develop.txt

This mandates mypy==0.641 and pytest==4.0.1. However, according to the tox.ini file, the requirements are actually mypy @ git+https://github.com/python/mypy@538d36481526135c44b90383663eaa177cfc32e3 pytest .

I guess tox.ini has the final word on this and I should install these versions ?

[altendky]

I'm not going to do a "complete" fixup of the requirements structure, but I am updating them over in https://github.com/python-qt-tools/PyQt5-stubs/pull/161. The mypy reference in tox was not great and is being removed. It was for some stubtest fix that wasn't released at that point in time. It shouldn't have been left like that without a note explaining it. In general, I try to keep things working with latest versions of dependencies.

Aside, I have had various issues where I can't recreate the CI results locally. It's been awhile so I forget how much I did figure out about that but I know some days I've just given up and gone off what CI says. This is certainly not a great state of affairs but I haven't yet prioritized addressing that.

[altendky]

Welp, I gave up on the reviews for #161 and merged it. I'll take the liberty to handle the catch up merge conflicts to avoid back and forth here. Just be sure to pull my changes.

[bluebird75]

This PR has been superseded by #153 .

Closing.