bpo-30801: Fix for shoutdown process error with python 3.4 and pyqt/PySide by alfonnews · Pull Request #2413 · python/cpython (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation13 Commits5 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Hello, and thanks for your contribution!
I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).
Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.
Thanks again to your contribution and we look forward to looking at it!
Alberto Fontán Correa and others added 4 commits
Fix for shoutdown process error with python 3.4 and pyqt/PySide bpo-1754: Fix for shoutdown process error with python 3.4 and pyqt/PySide
This has nothing to do with bpo-1754 (WindowsError messages are not properly encoded). Please reference the correct issue.
I think that I reference the correct issue.
Please correct me if it is not
You created an issue on Cython's issue tracker instead of CPython's issue tracker at bugs.python.org.
bpo-1754: Fix for shoutdown process error with python 3.4 and pyqt/PySide bpo-30801: Fix for shoutdown process error with python 3.4 and pyqt/PySide
This PR is not targeting the 3.4 branch; it's targeting master.
Python 3.4 is in "security fixes only" mode. Since this is not a "security fix", it will not be accepted into the 3.4 branch. Please either change the description so it doesn't mention 3.4, or close the PR.
If there are no changes to this issue in two weeks, I will close it.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be closed.
| sys.stdout.flush() |
|---|
| sys.stderr.flush() |
| #check if windows platform and Python 3.4 |
| if sys.platform.startswith('win') and sys.version_info[:2] == (3, 4): |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The version check makes this useless in any other version.
| @@ -18,6 +18,7 @@ |
|---|
| import signal |
| import itertools |
| from _weakrefset import WeakSet |
| import imp |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deprecated in 3.4. A patch using deprecated modules will not be accepted.
| imp.is_frozen("__main__")) # tools/freeze |
|---|
| if not is_frozen: |
| sys.stdout.flush() |
| sys.stderr.flush() |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fact that one frozen module crashes from the flushes does not mean that all frozen modules will or that suppressing flushes will not harm other frozen modules. The fact that other frozen modules do run on various versions of Python suggest that the problem is with the particular frozen module. This suggests to me that the patch is in general wrong and should be rejected now.
Ouch. Yeah, this code is awful and would never be merged.
@alfonnews: in the future, if you're interested in contributing to CPython, you should know that we have very high code quality standards. Your PR here is a "band-aid"; it doesn't fix the problem (whatever it is), it merely works around it. To be honest I still have no idea what problem you're solving. And the comments by @terryjreedy are all spot on.
I agree, in any case sorry for the inconvenience.
My intention was to expose the concrete problem and a casuistic that solves it, not as a final solution but as a starting point to detect the underlying problem and its correct solution.
I understand that it is not the right way for this. Thanks for corrections and suggestions