Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Way to a PyQt6-stubs release #3

Closed
bluebird75 opened this issue Feb 19, 2022 · 6 comments
Closed

Way to a PyQt6-stubs release #3

bluebird75 opened this issue Feb 19, 2022 · 6 comments

Comments

@bluebird75
Copy link
Contributor

Hi @TilmanK

I am not sure where to start with. We both have advanced a repository for PyQt6-stubs release. And we would both like to see a high quality stubs for PyQt6

On my side, the strategy and the guiding principle were :

  • PyQt5-stubs contains a lot of user contributed stub fixes, and I want to keep them
  • I also want user to continue to easily submit new stub fixes, giving inspirations for more automation
  • tests for stubs are a good idea. The more there are, the better. Ideally, every new user fix should come with a test.
  • PyQt6 releases will not change too much between two releases, so manual merging effort should be limited
  • automation is here to help where possible

With all these principles, I cloned PyQt5-stubs, checkout the PyQt5 upstream branch, committed the PyQt6 released stubs and merged the result into a PyQt6-stubs repository. After a long manual merge process, I have now reached a stable state. I have imported and fixed as well : the CI, the user contributed tests for stubs, running mypy on the stubs and even running stubtest. I also ran some of your automation scripts, the signal fixer and some of the automatic type: ignore annotater.

The end result is on my repo, pretty usable in my opinion. On the next PyQt6 release, I will have to perform a manual merge again, which I expect to be relatively painless.

On your side, you have gone a different road to reach a similar goal. If I understand correctly, there are zero manual modifications of the stubs, everything is automated so that you can go from one PyQt6 release to PyQt6-stubs release.

So now, the 1000$ question : can we work together on the same repo ?

Would you be OK to join my repo just by adding more automation scripts where it make sense, and still have a bit of manual merge ?

Would I be OK to join your repo ? Maybe. My main concerns are that I want the user to be able to report and fix incorect stubs. With the full automation approach, that means creating a python file to patch the stubs for each reported improvement. I find the approach rather tedious. Apart from that, CI and tests are easy to add, but you don't really need my help for this.

For the future of parsing the Qt documentation to generate solid input data, to create more automation, I'll be happy to work together with you on this,

@The-Compiler
Copy link

The-Compiler commented Mar 19, 2022

Hey @bluebird75 and @TilmanK! After meeting Tilman when I happened to be in Munich a few weeks ago, I talked about this topic with him a bit, and recently also chatted with @altendky about it.

While I admittedly haven't worked as much on the stubs as I hoped to, I believe it's me who advocated for (mostly) automated generation of stubs, e.g. over at python-qt-tools/PyQt5-stubs#6. Thus, let my try to add a couple of thoughts here.

Before I dive into possible solutions about what to automate and how exactly, let my try to first define what I believe the goals for PyQt[56]-stubs should be, because I think that should be something to be kept in mind: In an ideal world, we wouldn't have to do this at all! Ideally, at some point we would be able to upstream all our changes to PyQt itself, so that we don't need a lot of patching on top of those stubs,.

Now, there are some problems with that:

  1. The upstream stub generator (sip) is written in C, which is a language not everybody might be comfortable with. There is a Python rewrite planned apparently, but I have heard about that from upstream some 2 years ago, without any news since.
  2. PyQt upstream is an open source project with development methods stuck in the 2000s or so: There are source tarballs and a mailing list, but no public repository or bug tracker. While Phil (PyQt author) seems to be on board with improving the upstream stub generation, changes seem to require some patience and spoonfeeding. Saying "here is our changelog, please fix your stubs accordingly" simply isn't enough.
  3. Upstream's primary goal also seems to be IDE autocompletion rather than type checking, which is why the stubs are sometimes much simpler than they should be. Sometimes those two goals are at odds a bit (see e.g. Why is "incompatible with supertype" an error? python/mypy#1237).

While it's great to have alternative high-quality stubs, I feel like we shouldn't forget that it's probably worthwhile to do our adjustments in a way that it's still possible (and ideally, easy) to pick out a particular change, report that upstream in a clear and isolated way, and then remove it from our patching.

As a secondary goal, we of course want to continue offering our fixed stubs. We want to provide them for both PyQt5 and PyQt6, and need to somewhat frequently update them based on the latest upstream stubs.

Now to the part we seem to disagree on: How to best achieve those two goals. I feel like it's vital to get to a consensus here, as both @altendky and I don't really have the bandwidth to continue primary maintenance of the stubs. We'd be happy to give you both full access and host PyQt6 stubs under the "official" python-qt-tools namespace, but I feel like we need to resolve this topic first. The last thing we'd want is three slightly different PyQt6 stubs (including the upstream ones), that seems like a pain for everyone involved.

I believe it was originally my idea to automate everything, even the most trivial changes, in the spirit that @TilmanK has done from what I can see. Let my try to provide a bit of rationale why I believe this is a good idea.

My main problem when looking at PyQt5-stubs is that it's hard to see what kind of changes have been done overall, compared to the upstream stubs. The diff is pretty much useless, and the commit log isn't exactly useful to get an overview either, with over 1000 commits. There is an issues.md last updated in 2020, and there is a changelog which might be a bit more complete, but it's still much more incremental, rather than something to get a proper overview of the issues we fixed in our stubs (compared to the upstream ones).

Having full automation would give us a "single source of truth" of what our stubs are changing, which is 100% up to date, and can easily be modified to enable/disable certain fixes.

Why is that important? Because I believe it's the first step towards being able to categorize the issues with the upstream stubs, have a proper write up of what's wrong with them, and then to fix them upstream or at least report them in a way that they are going to be picked up (rather than a "here is a 30k diff" or "here is an incomplete partial fix which might demonstrate the issue").

Now, historically, people have done a lot of manual work on the stubs. They have fixed a few signals or enums here and there, manually. Many broken ones still remain. And after many such manual partial fixes of things, I lost track about what the problems actually are. I feel like we all lost track of trying to get things fixed upstream, and decided to maintain an independent fork of the stubs, for essentially forever.

I feel like "forcing" people to automate their fixes - rather than changing stub files by hand - does have many benefits. Yes, it might make it a bit harder to contribute initially, true. But it also means that a given issue with the stubs is fixed across the entire stubs, and in a way we can easily see what all the "sweeping changes" applied are, one by one. We can then proceed to report one of them upstream in a digestable way, and once it has been fixed there, we can simply drop that fix and regenerate things.

If we have manual (usually incomplete!) fixes and merging, we might be able to pick things from the changelog and report them upstream with an example of such a partial fix, then merge the upstream changes in the hope it won't conflict with our approach, and hope we don't accidentally undo a potentially different change when resolving the conflicts. Even if that might not have been a problem so far, once we actually attempt to get things fixed upstream, I feel like it might be something happening more often. At least in the past, we also lacked the bandwidth to do those merges, often leading to frustrating delays in releases when a new PyQt version came out.

With fully automated fixes, we can instead just run our fixer scripts over the stubs. We might even want to maintain one set of fixes, and then run them over both PyQt5 and PyQt6. Or older LTS versions of PyQt6 rather than the newest release (though probably that's not as important anymore, now that Qt 5.12 is out of support, and newer LTS releases are commercial-only).

From what I've seen in Tilman's code, it looks like it uses dataclasses to apply those fixes, with various predefined operations for common things to fix. I imagine the workflow to look something like this:

  • For sweeping changes (i.e. stuff with a diff more than a dozen lines or so): Write a new fixer, run it over the entire stubs. When happy with the changes, try to do a write-up and report them upstream, in the meantime, release a fixed versions of the stubs.
  • For single changes (i.e. a wrong upstream annotation for a single function or so): Add such a dataclass instance (basically a domain-specific language) to fix the issue in question. Much more importantly: Report it upstream. For this kind of change, it's likely fixable upstream in the code generator definition files (the .sip files defining the bindings), with no changes required to the code generator. It's also much more likely that this is just a mistake in the upstream definitions, rather than a result of the "type checking stubs vs. IDE autocompletion stubs" tension I mentioned above.

Thus, I believe when resisting the temptation in doing manual changes to the stubs, we will end up with better stubs (as we don't just fix something in one specific place, if something is an issue across all stubs). One-time changes might indeed be slightly more difficult to make, but those should definitely just be fixed upstream, and it seems like a worthwhile trade-off compared to the benefits.

Of course, we can get to those goals with the approach described by @bluebird75 too, which certainly has many less moving bits. I just believe it'll lead to contributors to do one-off fixes where a proper fix over the entire stubs (and/or reporting the problem upstream) would be far more appropriate. In the end, I feel like the most important thing is to find a way for all interested contributors to work together on the same code base.

@TilmanK
Copy link
Collaborator

TilmanK commented Mar 28, 2022

Sorry for not responding earlier, but I couldn't find the time.

To answer a few of @bluebird75's questions: Yes, there aren't any manually fixes in there. Everything that was fixed once in the PyQt5 stubs is fixed in this repo also (except deprecated changes), with the difference that it's automated. I don't believe that it's hard for someone to provide a fix – but I agree that it's currently not very well documented.

Apart from one thing, I totally agree with @The-Compiler. That one thing left is providing upstream fixes – simply because I've never thought about it.

But in the end, the stubs in this repo are as complete, as they could be. From what I understand, @bluebird75's sole concern is that contributions might be too complicated for other users, so I suggest we work on making them easier.

@bluebird75
Copy link
Contributor Author

Thanks for feedback.

First let me comment on the goal. Personally, my goal is to have high quality pyqt5/6 stubs available, so that anybody can write high quality type annotated python code while using PyQt5/6. I don't have the goal of contributing this back. The fact that Phil is not interested in stubs and that PyQt dev is still stuck in 2000s as you said certainly does not help. For me, it's perfectly ok that the stubs live their own life. The design of stub packages was done exactly to accomodate this kind of situation, and we are not the only python project where the stubs are provided separately (django comes to my mind but it's not the only one for sure). People looking for type annotation for their code will find this package easily, it is even strongly suggested by PyCharm when you start working with PyQt. Spare time to work on open source projects is scarce, so I would rather spend it making PyQt stubs better than trying to contribute them back to somebody who does not care.

That said, if somebody wants to spend time on this, he has my blessing.

Now, the second part : automation or no automation ? I personally find automation through git remembering merges to be fine but I'll let myself get convinced that the proposition of TilmanK is even better. I am skeptical but OK to go with it. And I would hate to have two pyqt6 stubs project living side by side. So, let's move on with TimanK's repository.

I do think that we will pay a price of complexity but maybe this is worth it. My request is that along automation, we create test cases of everything reported so that we keep a double trace (fix + reproducibility) of what we are doing.

I'll run a comparison of what I have against this repo to see if something is missing.

Addtionally, it would be nice to track the official raw PyQt6-stubs releases in a branch so that we can compare easily what has evolved.

And please write a small doc on how to contribute to the automation.

@bluebird75
Copy link
Contributor Author

We'd be happy to give you both full access and host PyQt6 stubs under the "official" python-qt-tools namespace, but I feel like we need to resolve this topic first.

I'll be more than happy to get this. Can you create the space and give access to both me and TilmanK ? We can then start to push for TilmanK's repository.

@bluebird75
Copy link
Contributor Author

PyQt6-stubs repo is now available. Discussion can be continued here if needed : #3

@altendky
Copy link

That link is to this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants