Issue32230
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2017-12-06 08:40 by ncoghlan, last changed 2022-04-11 14:58 by admin. This issue is now closed.
Pull Requests | |||
---|---|---|---|
URL | Status | Linked | Edit |
PR 4734 | closed | ncoghlan, 2017-12-06 10:27 | |
PR 4820 | merged | vstinner, 2017-12-12 21:40 |
Messages (17) | |||
---|---|---|---|
msg307709 - (view) | Author: Nick Coghlan (ncoghlan) * ![]() |
Date: 2017-12-06 08:40 | |
The `-X dev` option is currently integrated directly with the warnings module, rather than working indirectly through `sys.warnoptions`. This means that any third party code that currently checks sys.warnoptions will need to be updated to check sys.flags.dev_mode as well. Rather than doing that, I propose that we instead change the way dev mode works to *literally* be equivalent to `-Wdefault`, and remove the direct integration with the warnings machinery. (PR for that coming shortly) |
|||
msg307711 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2017-12-06 09:31 | |
I did it on purpose. -Wdefault adds the filter at the start, whereas -Xdev adds the filter at the end to respect BytesWarning. See tests in test_cmd_line and bpo-32089. |
|||
msg307712 - (view) | Author: Nick Coghlan (ncoghlan) * ![]() |
Date: 2017-12-06 09:49 | |
Note that this change will make "-X dev" effectively treat "-bb" and "-b" as equivalent, the same way "-Wd" already does. I figure that's OK, since it means the runtime behaviour will match the documentation (which says that "-X dev" implies "-Wd". |
|||
msg307713 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2017-12-06 09:56 | |
I was explicitly asked to not change the behaviour of BytesWarning with -X dev. -X dev documentation doesn't menton -Wd/-Wdefault anymore: https://docs.python.org/dev/using/cmdline.html#id5 "Warning filters: add a filter to display all warnings ("default" action), except of BytesWarning which still depends on the -b option, and use "always" action for ResourceWarning warnings. For example, display DeprecationWarning warnings." Another implementation issue is subprocess._args_from_interpreter_flags() which was unable to properly "rebuild" the -X dev option from sys.warnoptions. I mean that it's now simpler and safer with sys.flags.dev_mode. But I agree that the fact that -X dev doesn't touch sys.warnoptions is surprising and can cause issues. IMHO sys.warnoptions shouldn't exist in the first place. It looks like a hack to pass options from the C main() function to the warnings initilization code. But it's not like we can remove it right now :-) |
|||
msg307716 - (view) | Author: Nick Coghlan (ncoghlan) * ![]() |
Date: 2017-12-06 10:37 | |
It isn't good to have "-X dev" do magical things that can't already be replicated with other options. If we want "-bb" to take precedence over the passed in "-W" settings (which I agree would be a reasonable change), we should make that change universally, not restrict it to "-X dev". |
|||
msg307718 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2017-12-06 10:46 | |
> If we want "-bb" to take precedence over the passed in "-W" settings It's not only that. Without -b nor -bb, the default::Warning filter should still come after ignore::BytesWarning, since these warnings must be ignored by default. |
|||
msg307719 - (view) | Author: Nick Coghlan (ncoghlan) * ![]() |
Date: 2017-12-06 10:47 | |
I filed https://bugs.python.org/issue32231 to cover having -bb always override other warnings settings when it comes to handling BytesWarning. Writing that issue made me realise another quirk with `-X dev` though: as a command line option, it should really take precedence over PYTHONWARNINGS, while remain subordinate to other explicit -W options. I've adjusted the PR to implement that change, but haven't added a test case that ensures the behaviour persists. |
|||
msg307720 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2017-12-06 10:49 | |
First I proposed that -X dev behaves as -Wd -b, but Antoine Pitrou and Serhiy Storchaka asked to remove -b: https://mail.python.org/pipermail/python-dev/2017-November/150517.html https://mail.python.org/pipermail/python-dev/2017-November/150519.html https://mail.python.org/pipermail/python-dev/2017-November/150520.html |
|||
msg307721 - (view) | Author: Nick Coghlan (ncoghlan) * ![]() |
Date: 2017-12-06 10:50 | |
No, we don't need to worry about BytesWarning if neither -b nor -bb are set, as the interpreter doesn't even emit the warning in that case. The only thing the warnings machinery gets used for is to print the messages and/or raise the exceptions (so it would arguably be clearer to omit the filter entirely in the case where the flag isn't set at all). |
|||
msg307723 - (view) | Author: Nick Coghlan (ncoghlan) * ![]() |
Date: 2017-12-06 10:54 | |
In relation to the utility of sys.warnoptions: it also lets other modules (like the unittest module) inspect the warnings that have been specified on the command line. If it wasn't for that use case, I wouldn't be concerned about the fact that `-X dev` doesn't currently set it. |
|||
msg307724 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2017-12-06 10:56 | |
> It isn't good to have "-X dev" do magical things that can't already be replicated with other options. While it might be nice to be able to replicate -X dev "manually", I didn't try to implement that. The current implementation of this option is more complex than what I expected, especially for warnings. > Writing that issue made me realise another quirk with `-X dev` though: as a command line option, it should really take precedence over PYTHONWARNINGS, while remain subordinate to other explicit -W options. IMHO -X dev should change the default, but let the developer overrides options set by -X dev. For example, -X dev -bb works as expected: raise BytesWarning, and this is a test for that. There is also a test for -X dev -W error: -W error has the priority over -X dev, I did that on purpose. When I started to implement -X dev, I didn't expect so many corner cases. The problem is that the code reading the "configuration" (command line arguments, environment variables, a few configuration files, etc.) was spreaded around CPython code. You know that very well, since you wrote the PEP 432 :-) Fix the implementation of -X dev was my first motivation to work on the implementation of the PEP 432: bpo-32030. For PYTHONWARNINGS vs -X dev, I'm not sure. It's rare to use PYTHONWARNINGS. I put PYTHONWARNINGS and -W options at the same level. If someone uses PYTHONWARNINGS, warnings are likely already well understood. Overriding PYTHONWARNINGS with -X dev can be seen as a bug. Sometimes, you cannot set command line options, only environment variable. There is -X dev, but there is also PYTHONDEVMODE=1 ... |
|||
msg307727 - (view) | Author: Nick Coghlan (ncoghlan) * ![]() |
Date: 2017-12-06 11:28 | |
Right, I hit some of those corner cases myself before I realised that pymain_add_warnings_options was the right place to inject "default" into sys.warnoptions after your refactoring improvements. It provides a nice pivot point where the command line machinery can decide relative priority and turn things into a neatly ordered list. That gives the following priority order in my PR: - first we add the filters from PYTHONWARNINGS - then we optionally add an extra "default" entry based on core_config->dev_mode (which may be from either PYTHONDEVMODE or from "-X dev") - then we add the filters from "-W" command line options The "last added, first applied" behaviour of "warnings.simplefilter" then means that later entries in that list take precedence over earlier entries. If we go on to also implement https://bugs.python.org/issue32231, then we'd add a 4th step to inject a "default::BytesWarning" or "error::BytesWarning" filter at the end based on cmdline->bytes_warning. |
|||
msg307730 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2017-12-06 11:48 | |
I prefer to not talk about sys.warnoptions since, as you wrote, the order is reversed. IMHO the priority should be (high priority > low priority): -W options > PYTHONWARNINGS > BytesWarning > -X dev > default filters With: * BytesWarning: "ignore" by default, "default" for -b, "error" for -bb. * default filters: 4 filters added in release mode [('ignore', None, <class 'DeprecationWarning'>, None, 0), ('ignore', None, <class 'PendingDeprecationWarning'>, None, 0), ('ignore', None, <class 'ImportWarning'>, None, 0), ('ignore', None, <class 'ResourceWarning'>, None, 0)] or 1 filter for pydebug build: [('default', None, <class 'ResourceWarning'>, None, 0)] Currently, only "-W options > PYTHONWARNINGS" goes into sys.warnoptions, whereas "BytesWarning > -X dev > default filters" is added by the warnings module ("init_filters()"). |
|||
msg307731 - (view) | Author: Nick Coghlan (ncoghlan) * ![]() |
Date: 2017-12-06 12:01 | |
If we wanted to go that way, then I'd go back to my original code order for - putting the '-X dev' setting first in sys.warnoptions, so both PYTHONWARNINGS and -W options override it. Then https://bugs.python.org/issue32231 would determine the relative precedence of the BytesWarning filter. (I still think the interaction between "-Wd" and "-bb" is weird, but we can argue about that on the other issue). |
|||
msg307732 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2017-12-06 12:02 | |
"-W options > PYTHONWARNINGS > BytesWarning > -X dev > default filters" Hum, sorry, according to bpo-20361 (and bpo-32231 which has been marked as a duplicate of bpo-20361), -b and -bb options must have the priority over -W/PYTHONWARNINGS. So it gives something closer to: -b and -bb > -W > PYTHONWARNINGS > ignore::BytesWarning > -X dev > default filters where ignore::BytesWarning is only not added if -b or -bb is used. |
|||
msg308156 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2017-12-12 21:59 | |
New changeset 747f48e2e92390c44c72f52a1239959601cde157 by Victor Stinner in branch 'master': bpo-32230: Set sys.warnoptions with -X dev (#4820) https://github.com/python/cpython/commit/747f48e2e92390c44c72f52a1239959601cde157 |
|||
msg308159 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2017-12-12 22:04 | |
We found an agreement with Nick :-) I applied Nick's PR. The issue is now fixed: vstinner@apu$ ./python -X dev -c 'import sys; print(sys.warnoptions)' ['default'] This PR also fix bpo-20361: -b and -bb options now have the highest priority. The documentation is maybe not perfect, but Nick stated that he will update the doc with the implementation of his PEP 565. |
History | |||
---|---|---|---|
Date | User | Action | Args |
2022-04-11 14:58:55 | admin | set | github: 76411 |
2017-12-12 22:04:27 | vstinner | set | status: open -> closed resolution: fixed messages: + msg308159 stage: patch review -> resolved |
2017-12-12 21:59:50 | vstinner | set | messages: + msg308156 |
2017-12-12 21:40:02 | vstinner | set | pull_requests: + pull_request4710 |
2017-12-06 12:02:56 | vstinner | set | messages: + msg307732 |
2017-12-06 12:01:24 | ncoghlan | set | messages: + msg307731 |
2017-12-06 11:48:33 | vstinner | set | messages: + msg307730 |
2017-12-06 11:28:32 | ncoghlan | set | messages: + msg307727 |
2017-12-06 10:56:40 | vstinner | set | messages: + msg307724 |
2017-12-06 10:54:44 | ncoghlan | set | messages: + msg307723 |
2017-12-06 10:50:06 | ncoghlan | set | messages: + msg307721 |
2017-12-06 10:49:05 | vstinner | set | messages: + msg307720 |
2017-12-06 10:47:48 | ncoghlan | set | messages: + msg307719 |
2017-12-06 10:46:56 | vstinner | set | messages: + msg307718 |
2017-12-06 10:37:12 | ncoghlan | set | messages: + msg307716 |
2017-12-06 10:27:57 | ncoghlan | set | keywords:
+ patch stage: test needed -> patch review pull_requests: + pull_request4637 |
2017-12-06 09:56:25 | vstinner | set | messages: + msg307713 |
2017-12-06 09:49:48 | ncoghlan | set | messages: + msg307712 |
2017-12-06 09:31:37 | vstinner | set | messages: + msg307711 |
2017-12-06 08:41:00 | ncoghlan | create |