Add all qflags bases classes

[bluebird75]

I started from my work on windowFlags and look for ways to make it generic. I failed at defining a generic approach with mypy, protocols and generics inheriting from int, it is really above my understanding of mypy.

But, it's easy enough to generate a file for each QFlag based class which can be used to assert both the actual implementation types and values, and which ensures that the class is being correctly checked. This is what I am doing here for two classes : WindowFlags and Alignment.

With a few more hours, I should be able to cover all qflags based class. This would remove a big hole in PyQt stubs.

[bluebird75]

The file where I would be interested for your review is qflags_aligmentflags.py . It uses a mostly generic approach on QFlag and QFlags based classes to check all operations on both mypy and python side.

Only two lines are specific to AlignementFlag in this file. I have work in progress to generate one similar file per QFlag based class (for the testing side) and applying automatically a fix to the pyi files to add the missing methods.

[bluebird75]

Thanks, this is exactly the feedback I am looking for.

The use of globals was rather convenient when focusing on getting things done. I agree with you that it's better without it. Fully ok for using dataclasses, it certainly makes the thing more readable.

Just curious, how do you avoid the if __name__ == '__main__' ?

The sourcePart2 is indeed captured but not outputed anywhere. It is the part which is replaced.

For the full generation approach, I was thinking along the lines of :

1. generate the qflag based test class
2. patch the relevant .pyi file
3. run local py.test + mypy test
4. if successful commit test file + updated pyi file, if fail, just skip the change as of now
5. move to next qflag class

One consequence of this generic approach is that there will be one test file par QFlag class, so expect quite some test files. Is that OK with you ? Or do you consider them ony as a local validation tool which does not need to go into git ?

[altendky]

The use of globals was rather convenient when focusing on getting things done. I agree with you that it's better without it. Fully ok for using dataclasses, it certainly makes the thing more readable.

Just for the record, I almost exclusively use attrs, but didn't feel like bothering with adding the dependency here right now. Might get swapped over later.

Just curious, how do you avoid the if __name__ == '__main__' ?

In this case you just remove it. We aren't trying to import the file so there's no need to avoid the function being called if we were to import the file. In general, I put all modules in the package and then use entry points to provide the ability to run from the command line. I rarely have directly runnable .py files in the projects I work on.

This is a test helper with the tests outside the package which makes it not fit this mold so well. That's part of why I'm not pushing the matter here.

Why shouldn't we use if __name__ == "__main__":? One issue is that it succeeds at allowing you to do what it is intended to. Both import and directly run the same file. In many cases this isn't a problem but when both happen in the same process you end up with two modules created from the single file. It gets pretty confusing when there are two classes created from the same code that are actually different types. I think I have an example on repl.it of this. I can try to dig it up if you are interested.

The sourcePart2 is indeed captured but not outputed anywhere. It is the part which is replaced.

That's probably worth at least a comment, or just not collecting that part at all.

For the full generation approach, I was thinking along the lines of :

1. generate the qflag based test class
2. patch the relevant .pyi file
3. run local py.test + mypy test
4. if successful commit test file + updated pyi file, if fail, just skip the change as of now
5. move to next qflag class

One consequence of this generic approach is that there will be one test file par QFlag class, so expect quite some test files. Is that OK with you ? Or do you consider them ony as a local validation tool which does not need to go into git ?

@The-Compiler, perhaps you want to chime in since you have been talking about a more automated handling of these stubs for awhile now?

I suggested not putting the generated test files in git since they are generated and could be regenerated before each test. I could go either way though. But if they are committed, then CI should tell us if regeneration would change them.

[bluebird75]

The script is now ready. It was a bit more difficult than I expected but nothing too fancy. It is now 1000 lines, with a proper README. I will start committing some flags and see how it goes.

One issue which I am already encountering is that not all QFlags are equals. I encountered some small difference, which where caught by the strong test script. The script assumes one behavior and rejects everything else. This is fine for now, I'll look into the rejected cases another time to understand the behavior difference.

[bluebird75]

After some review, archiving a test file for each added QFlag is too much. There will be close to 100 test files just for the 10 modules listed today. When adding all other QFlags from all other modules, we should be close to 300 extra test files. This will just slow down the CI without any significant benefit. Maybe a lighter form to verify that the flag is properly supported should be the target, like checking one or two operations per flag.

[altendky]

Do the tests actually take a long time?

[bluebird75]

I was expecting a big impact but actually, after grouping all the qflag checks in one call, it is only 15 second impact. So very reasonable.

The branch is close to be ready for a merge. Anything else you want to see adjusted ?

[bluebird75]

This is all green finally. Code is ready to be merged.

[bluebird75]

Hi Kyle. Can you start reviewing that one ? It is the result of a long work so I would be thrilled to have it integrated.

[bluebird75]

Thanks for the feedback. This read_text() was unknown to me and I appreciated the other ideas. The code was not written with top standards but to achieve a useful purpose. It probably could be improved.

It is not really feasible to run the generation on CI. First, it is very time consuming because libcst needs to parse the huge PyQt5 modules for each QFlag generation. Second, the source of the QFlag information is a manual grep over Qt sources, which requires some user modifications before being a proper source for complete generation.

But it's a good idea to run it on new Qt or PyQt releases.