| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
One paranoid comment, but overall seems reasonable.
Sorry, something went wrong.
|
|
||
| @functools.wraps(__init__) | ||
| def wrapper(self, **kwargs): | ||
| QtWidgets.QWidget.__init__ = cooperative_qwidget_init |
There was a problem hiding this comment.
This needs to be in a try..finally block.
Sorry, something went wrong.
There was a problem hiding this comment.
done
Sorry, something went wrong.
There was a problem hiding this comment.
The initial monkey-patch should also be in the try block... ctrl-c can happen anywhere.
Sorry, something went wrong.
There was a problem hiding this comment.
Wow, I'm a bit ashamed I never thought about this point :-)
Sorry, something went wrong.
|
I lean towards this as the better solution (I started writing a comment about how I was even between them and in the process convinced my self to prefer this one). This has the advantage of being cleaner and more localized but hard-codes the non-cooperative modules in. However, odds of another binding showing up are low and doing the sub-classing and replacing __init__ while waiting for changes to be merged upstream isn't that bad. #9048 uses 'simpler' python, but it makes the class zoo one class bigger and has to use two differently named 'magic' cut-out function, and is spread across a couple of files. It makes the unlikely extension case easier (once you understand the code), but harder to understand to begin with. |
Sorry, something went wrong.
There was a problem hiding this comment.
Be more paranoid in re-setting the __init__
Sorry, something went wrong.
|
try... finally added. |
Sorry, something went wrong.
|
Appveyor failure is just a hang at the start of the conda-build on 3.6. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Alternate for #9048 (basically pulling out the relevant parts of #8771, but using a local monkey-patch approach.
I personally find this approach clearer in intent ("PyQt4 does not support multiple inheritance so let's just locally patch it so that it does") than #9048.
PR Summary
PR Checklist