python-qt-tools / PyQt5-stubs

Stubs for PyQt5
GNU General Public License v3.0
68 stars 31 forks source link

Plans for PyQt6? #69

Closed BryceBeagle closed 2 years ago

BryceBeagle commented 4 years ago

PyQt6 snapshots just came out. Currently they're nothing more than PyQt5 with some deprecated features removed. But it begs the question: will PyQt5-stubs support PyQt6?

altendky commented 4 years ago

I haven't looked hard at 6 but it sounds like there are the expected backwards incompatible changes you would expect from a major version, even when wrapping Qt5. (the enum stuff?) I imagine the answer being 'no it does not' but that it would be nice to. I have no idea on the timeline for Qt6 but i tend to like moving forward sooner than later so I will likely be on-board for working on the support. But for now I think getting cleaned up with 5 and seeing if we can work with Phil to get proper fixes upstream would be my priority. Then there's less for us to deal with (theoretically nothing, but...).

BryceBeagle commented 3 years ago

PyQt6 was just officially released: https://www.riverbankcomputing.com/pipermail/pyqt/2021-January/043485.html

Also of interest is that the OAuth stuff is now moved to its own package, PyQt6-NetworkAuth. The stubs would probably need to account for this

TilmanK commented 3 years ago

Guys, I just migrated one of my projects to PyQt6. No stubs. It's horrible ;)

Do you have any plans so far? Getting the PyQt5 stubs at least to work isn't that hard (tried it here: https://github.com/TilmanK/PyQt5-stubs), nevertheless, I think that you generated them somehow automatically in the past?

I know that they changed some arguments (pos is now position at some places), you need to use Enums now instead of class properties (i.e. QEvent.Type.Close instead of QEvent.Close) - so the PyQt5 hints aren't that useful either. Additionally, to what @BryceBeagle already wrote above.

TilmanK commented 3 years ago

I dug a bit and found that the generated stubs come with PyQt6 itself. So this is basically just a lot of manual work to merge them together, right?

Before I waste my weekend on this: Have you done anything yet for PyQt6? How do you plan on integrating it into this repo (do you even? - doesn't seem like the best thing to do)...

altendky commented 3 years ago

PyQt5 also came with stubs towards the end, but a lot of wrong stuff (and maybe some preference too). I think Phil got some things fixed in PyQt6, but it seemed like they wanted an item by item list of each issue and I didn't bother with that (beyond being able to diff against ours here and also our changelog). So, I expect there's a lot more work to be done with the stubs for 6. I haven't gotten to looking though. That said, I am rolling my way through Qt 6 support (https://github.com/python-qt-tools/qts for example) so maybe I'll get back to the stub layer.

Should we just move forward and update the upstream etc branches to six? If there's another PyQt5 release we can go back and branch off and deal with duplicative branches at that point (or just skip it and move on).

TilmanK commented 3 years ago

I don't know if it's a good idea to just update master, yet. From what I know, there are still some issues to be fixed in PyQt5 and PyQt5 we will be around for some time for sure.

I'd personally opt for a branch from which we can create a version exclusively for PyQt6. Supporting both versions at the same time will surely be a mess.

I'd be willing to provide an initial PR for that. What do you think?

altendky commented 3 years ago

Sorry, I didn't mean mixing the two together. I think the only difference in our ideas is whether we pre-emptively create separate branches for 5 and 6, or just move on to 6 and then create maintenance branches for 5 if we end up working on it again.

Anyways, the help would be appreciated. I think it's safe to go ahead and start a PR and we can redirect it to a different branch depending on what we decide? Are you familiar with the upstream branch that has exactly upstream stuff and is where we do the builds vs. the master branch where we maintain all our modifications?

ping @The-Compiler, @BryceBeagle

altendky commented 3 years ago

Mmm... I guess it would be nice to make it through the PRs and get a release out for that.

The-Compiler commented 3 years ago

Given that this repo/project is called PyQt5-stubs, IMHO we should even consider having a separate PyQt6-stubs repo. OTOH this would probably duplicate a lot of the CI infrastructure and such.

Since we essentially have a "fresh start" with the PyQt6 stubs, I still wonder if we can somehow start to automate the stub rewriting instead of doing it all manually (see #6). It looks like nothing has changed about the SIP stub generator part being written in C so far, so IMHO the best way to approach this would be parsing and rewriting the stub files (perhaps using something like LibCST.

I'd really like to find some time to tackle this myself, but probably that's just not going to happen in the next 6 weeks or so. I've talked with @RJ722 about this and he mentioned he'd be interested - maybe now would be the time? Happy to mentor a bit, if you want to give this a try.

altendky commented 3 years ago

I guess there's the option of just putting in the time upstream with PyQt6? Basically take the checking mechanisms and tests here and just get the official stuff right.

TilmanK commented 3 years ago

I like @The-Compiler's idea, but isn't that exactly what's our problem all the time? We don't want the stuff from the official files, because it's full of errors?

Wouldn't it be smarter to create a diff between two versions of the official stubs and create something from there?

TilmanK commented 3 years ago

On the other hand, thinking about it, that's exactly what we want. If we ignore the stubs completely, we could build a functional diff and work on things from there...

The-Compiler commented 3 years ago

I don't think it's useful to build a diff between PyQt5 upstream and our PyQt5 stubs and then apply that to the PyQt6 stubs. The problem so far has been that we've always fixed things on a case-by-case basis manually, rather than taking a more structured and automated approach.

If we had a more structured way of fixing problems (e.g. "take all signals and change their annotation from a function to a signal type"), we could fix all such issues rather than just a small part of them, and we could do so in a way it's easier to maintain, and easy to apply to both PyQt5 and PyQt6 upstream stubs, as well as easy to update for every new release of those.

TilmanK commented 3 years ago

I think we're talking about two things here. What I was thinking of, is to compare the current upstream version (independent of its mayor version) to our stubs so that we would be able to generate a diff of what has changed (added, removed methods, classes, arguments and so on).

What you suggest is useful independent of my thoughts ;)

Nevertheless, what is the right way to upgrade to PyQt6 apart from figuring out the differences (automatically or manually) and applying them to the current code base?

The-Compiler commented 3 years ago

There's an upstream branch in this repo, but the diff is going to be huge: https://github.com/python-qt-tools/PyQt5-stubs/compare/upstream..master

As well as some notes on what issues there are: https://github.com/python-qt-tools/PyQt5-stubs/blob/upstream/issues.md - but I doubt they are up to date currently.

TilmanK commented 3 years ago

I played around yesterday evening a bit and compared the PyQt6 upstream stubs with our stubs. Here's the result so far:

Apart from that, they did a good job, at least for the stuff I saw in QtCore. I guess those Enums are probably (at least for my part) something that will help a lot...

altendky commented 3 years ago

What would we do about Liskov warnings other than silence them? I would presume that Qt isn't going to "fix" their API.

Maybe I have missed the response, but isn't the "correct" approach to work with Phil to get the stubs in the officially released wheels proper? To help with setting up tests that show the issues and optionally solutions to the existing stub generator system.

And yeah, the diff is kind of funny. It's big. Then you realize that the 21 files with the biggest diff aren't even expanded... (at the above provided link)

TilmanK commented 3 years ago

The Liskov warnings need to be silenced, yes. The point is, that someone has to do it 😀

I've merged together our current stubs with the PyQt6 upstream stubs (for QtCore). I've tried to come as close as possible to the PyQt6 thing so a diff will quickly reveal our fixes and changes. I'll try to do a PR in the evening since I need some feedback and want to discuss some things.

And btw: Who is Phil? Is he one of the PyQt devs? ☺️

The-Compiler commented 3 years ago

Maybe I have missed the response, but isn't the "correct" approach to work with Phil to get the stubs in the officially released wheels proper? To help with setting up tests that show the issues and optionally solutions to the existing stub generator system.

Yes, for issues where this makes sense - IIRC Phil's position has been that the goal of the stubs is mostly to assist for IDE autocompletion (rather than type checking), and for some issues (like the Liskov substitution problem) there's probably no way to make the stubs compatible with mypy while still making them as useful as possible for IDEs.

Also, this might take some time, and we might need to send patches - so if we had an automated way to do those fixes in the meantime, IMHO that'd be a great starting point either way.

And btw: Who is Phil? Is he one of the PyQt devs?

More like the PyQt dev - that's Phil Thompson from Riverbank Computing. I'm not aware of any other person working on PyQt.

TilmanK commented 3 years ago

Ok, I just tried the solution @The-Compiler suggested (rewrite the signal definitions using LibCST) - which was, in the end, quite simple. I can fix the signals ad-hoc for the PyQt5-stubs, could also do the same for the upstream stubs.

I guess it's also possible to fix some other problems automatically (Liskov principle violation, maybe use of QByteArray annotations, some other stuff from https://github.com/python-qt-tools/PyQt5-stubs/blob/upstream/issues.md maybe as well).

But in the end, I still don't see how to bring this all together? I can use that script to generate better stubs for PyQt6 than they are upstream, but how do I merge all the stuff that was fixed by hand?

TilmanK commented 2 years ago

So here's the thing that @The-Compiler requested, an automated way of generating/fixing the stubs: https://github.com/TilmanK/PyQt6-stubs

It downloads PyQt6 from pypi, extracts the stubs and fixes them, leaving us with a diff of what really changed after the fixes. It's ugly, not complete, not very performant - I just tried to make it work and that's what it does for now. I'm at 50ish mypy warnings for the complete stubs where I did not have the time to look into, but most of the generic stuff is fixed (signals, overloads, slots etc.).

I'm happy to somehow integrate this here or to continue to work on my repo. Furthermore, I'd be happy to receive some feedback from you on how to proceed. Like I said, this is far from complete and production ready - and it's yet not decided if this will continue to live somewhere.

But at least it lets me migrate some projects to PyQt6.

bluebird75 commented 2 years ago

I would like to revive this discussion, now that I am an active PyQt5-stubs developer and looking for a PyQt6-stubs release.

One of the debate is whether we shall automate our stubs so that we can go from a clean released stub of PyQt5/6 to what we have today, to facilitate upstream updates. Or whether we can continue relying on git and manual merges.

I believed that our current mixed approach is good. I personally don't believe that it is possible to fully automate the stub transformation because some information is simply not available for automation. For automating the QFlag adjustements, my input was a grep in qt sources, so quite manual as a start. But eventually automated when generating a hundred of test files with slightly different behaviors. Another nice improvement is marking None as accepted parent widget parameter. We have it from a few PR of users, but this can not be automated easily : the information is only available in Qt's documentation.

That said, everything we can automate, we should automate it. We have now two automation scripts. Both of them check if the change is already applied and apply it if that's not the case. That's the way to go, automation resilient to already applied situation. In my case this make the script really more complicated but I find the price acceptable.

My other comment is that thank to git, there is a trace on how a conflict was solved between evolving upstream and our stubs, This means that for a new upstream release, the amount of manual fixes should be very low. This was the case for the last upstream PyQt5 releases.

My personal take on this is that we should go the @TilmanK way, going through an intial significant merge and then enjoying git to help deal with upstream evolutions. In parallel, we should document what automation we have and how to apply it to new classes / modules being added to upstream (although that remains a rare event).

TilmanK commented 2 years ago

I would like to revive this discussion, now that I am an active PyQt5-stubs developer and looking for a PyQt6-stubs release.

One of the debate is whether we shall automate our stubs so that we can go from a clean released stub of PyQt5/6 to what we have today, to facilitate upstream updates. Or whether we can continue relying on git and manual merges.

I'd really appreciate a release to pypi for the PyQt5-stubs first, the last one is from February. ;)

I believed that our current mixed approach is good. I personally don't believe that it is possible to fully automate the stub transformation because some information is simply not available for automation. For automating the QFlag adjustements, my input was a grep in qt sources, so quite manual as a start. But eventually automated when generating a hundred of test files with slightly different behaviors. Another nice improvement is marking None as accepted parent widget parameter. We have it from a few PR of users, but this can not be automated easily : the information is only available in Qt's documentation.

That said, everything we can automate, we should automate it. We have now two automation scripts. Both of them check if the change is already applied and apply it if that's not the case. That's the way to go, automation resilient to already applied situation. In my case this make the script really more complicated but I find the price acceptable.

You don't want to merge/diff from PyQt6 upstream to this repo. You don't. Honestly, I tried, it was full of pain.

Actually, I've shown some examples on how to automate stuff and how to "process" new PyQt6 releases, you might want to take a look first at what I did (it's dirty as hell, but works).

I didn't write stubs or merge anything, I wrote an automation to get the upstream stubs and patch them. This frankly means that with every PyQt6 release I delete everything I have, download the upstream stubs, fix them and then overwrite them (with the bonus of a clear git diff). Which means: I don't need to merge :)

bluebird75 commented 2 years ago

I had a look at your stuff, nice job. But on the other hand, you are missing all the manual improvements done recently, which is a pity.

I still believe in huge merge effort, but maybe I underestimate the workload.

The-Compiler commented 2 years ago

Another nice improvement is marking None as accepted parent widget parameter. We have it from a few PR of users, but this can not be automated easily : the information is only available in Qt's documentation.

I'd stay it still can be automated - with that automation relying on a manually curated data source. In other words, there could be a JSON file or something with a list of everything that needs a None addition, but the addition itself is then still fully automated so that it can easily be applied at a new release of the upstream stubs.

TilmanK commented 2 years ago

From a very quick and lazy look at the stubs I haven't seen a problem with the parent argument types on the widgets and I also haven't seen a problem when using them.

Nevertheless, if they are wrong - we might be able to generate that source @The-Compiler is talking about with a web scraper from the docs or with some other method directly from the sources' doc. Another thing that we could use as data source: This repo.

I agree, this should be automatable.

bluebird75 commented 2 years ago

It is very similar in spirit to what I did with QFlags derivative. The information about a class being a QFlag is not available directy in PyQt5. In this case, I grepped Qt sources for QFlag declaration. I then turned it into a big json file containing very basic information of the type "class QStuff is a QFlag". Then, with automation and a few manual adjustments, I was able to locate the QFlag class precisely in PyQt5. The rest was then fully automated to patch the PyQt5 stubs and generate test data.

The same process can certainly be repeated for other topics, using Qt documentation scraping as source instead of Qt source grepping. It is not trivial but not rocket science either.

I plan to work on such topic ... after a PyQt5-stubs release and a decent PyQt6-stubs release.

But even with these automation, there are user contributed improvements in PyQt5-stubs which we can not catch. That's why I keep believing in a mixed approach: automation + user contributions.

bluebird75 commented 2 years ago

I have PyQt6-stubs working. It is still work in progress but I have so far :

The CI is running green on Python 3.6 on Windows.

Next steps are :

It's all done here : https://github.com/bluebird75/PyQt6-stubs

I would like to move the repo eventually to python-qt-tools/PyQt6-stubs .

TilmanK commented 2 years ago

I'd stay it still can be automated - with that automation relying on a manually curated data source. In other words, there could be a JSON file or something with a list of everything that needs a None addition, but the addition itself is then still fully automated so that it can easily be applied at a new release of the upstream stubs.

@The-Compiler You're right. Here's the proof: https://github.com/TilmanK/PyQt6-stubs. It's completely automated with LibCST, I dropped the part where I edit files on str-basis and moved that to LibCST too. I'm storing the fixes in dataclasses (you could use JSON too, doesn't matter) and apply them on the run. Point is, you run generate_upstream.py and you'll get exactly what's currently in the repo.

I'm missing some errors from mypy (~ten), but it doesn't make any sense to me putting an ignore comment behind them instead of fixing them - so I'll do that in the next days.

TilmanK commented 2 years ago

Ok, I fixed the rest of the errors in mypy. @bluebird75: Do you want to contribute the CI-stuff you already made?

I'll put the stuff on pypi soon...

bluebird75 commented 2 years ago

@TilmanK Can you contact me be email ? It will be easier to discuss this. My email : phil.fremy at free.fr

bluebird75 commented 2 years ago

Discussion shall continue on the pyqt6-stubs repo