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
DeprecationWarnings should be visible by default in the interactive REPL #68482
Comments
DeprecationWarning and PendingDeprecationWarning are invisible by default. The rationale for this is that they are only useful to people who are writing code, so they should not be shown to end-users who are merely running code. If someone is typing stuff into the interactive REPL, though, and the code they type uses some deprecated functionality, then we should actually show them this warning. We know that the author is sitting right there. And they're probably going to take whatever they tested interactively and move it into a more permanent form and ugh. This problem is particularly acute for packages that are primarily used through the interactive prompt, like numpy. I am tired of getting bugs like this: numpy/numpy#5919 The solution is simple: when entering interactive mode, the REPL should do something like: warnings.filterwarnings("default", category=DeprecationWarning, module="__main__")
warnings.filterwarnings("default", category=PendingDeprecationWarning, module="__main__") |
I also filed a similar bug with ipython: ipython/ipython#8478 |
I have learnt to run the interactive interpeter (and also most of my own scripts) with the -b -Wall options. But having these switched on automatically may not be a bad thing. |
See discussion on Python-Ideas [1]. |
+1 for showing DeprecationWarning by default (as these features may be going away in the next X.Y release of Python) -0 for showing PendingDeprecationWarning by default (as these won't be going away until X.Y+1 at the earliest) |
*cough* You know, there's more to life than Python-X.Y.tar.gz :-). Not that I know how PendingDeprecationWarning is used in the wild. I've been thinking maybe we (numpy) should start using it for stuff that we want to discourage people from using (we know it was a bad idea or intrinsically broken or whatever), but don't yet have a full replacement to offer. It would help if there were some official guidance on what these things mean -- I can't find anything written down anywhere that even documents what you just said about how CPython proper uses them, so I imagine people have come up with all kinds of interpretations. Anyway, my logic would be: when I am trying something out at the REPL, usually it is because I want to check how it works, to figure out what I should be doing in the real module code that I'm writing in the next window. When doing this, I definitely appreciate knowing that it will be removed and is being replaced -- it won't necessarily stop me from putting it into my code, but at least it's information that I can take into account. I don't care a huge amount either way, though; if we decided to hide PendingDeprecationWarnings by default while showing DeprecationWarnings then numpy would just define a NumPyPendingDeprecationWarning subclass of DeprecationWarning and use that instead of PendingDeprecationWarning and all would work out fine :-). |
The difference between the two used to be clearer: prior to Python 2.7, PendingDeprecationWarning was hidden by default (and thus mainly only visible to folks testing with -Wall), while DeprecationWarning was visible by default. We blurred the line between the two thoroughly when DeprecationWarning also became hidden by default, giving the status quo: Test frameworks: both visible by default Splitting them in the interactive REPL case would restore a meaningful behavioural difference that can help pragmatically guide decisions as to which is more appropriate to use: Test frameworks: both visible by default The current unanswerable "How do my users interpret the difference between DW and PendingDW?" question would be replaced by the much simpler "Do I want folks using the interactive REPL to see this warning or not?" |
Okay, that sounds reasonable to me. |
For Idle, the addition could be made in current versions. Idle compiles user code in the idle process and ships it to the user process for execution. In particular, idlelib.run.Executive.runcode, line 351, is Please post test cases to enter by hand, both at the prompt and in the editor, to get system warnings for 2.7 and 3.4 or 3.5. Also, please copy the positive and negative examples you posted to python-ideas. |
There isn't really any magic in how warnings work. Basically someone calls warnings.warn(...), which is a regular Python function, and it gathers up some information about the specific warning message and calling context, checks a global variable ("warnings.filters") to decide how this message in this context should be handled, and then either does nothing, prints something to stderr, or raises an exception. There are lots of warnings that are printed by default, and I'm sure IDLE is handling them fine already. It's just (Pending)DeprecationWarnings in particular that have an entry stuck into warnings.filters saying "please ignore these" (unless it gets overridden by something else). So you just need to make sure that a filter is added to warnings.filters that says to treat DeprecationWarnings generated by the __main__ module using the "default" action. ("default" is the name of a specific way of handling warnings; for most types of warnings, the default handler is the one named "default", but for DeprecationWarning, the default handler is the one named "ignore". Obvious, right?) So you just need to make sure to run the following line of code somewhere in the user process: warnings.filterwarnings("default", category=DeprecationWarning, module="__main__") Adjust as necessary if (a) you want to apply similar handling to PendingDeprecationWarning, (b) your user namespace has some value of __name__ that is different from "__main__". Then to test, you can just type warnings.warn("foo", DeprecationWarning) at the prompt, and it should be printed. Note that because the warnings module tries to suppress duplicate warnings (which is good), and it has a bug where it can't tell the difference between different lines of code at the REPL (this is bad -- see ipython/ipython#6611 , and there should probably be a python bug too but I haven't gotten around to filing it), then the *second* time you run that line of code in the same REPL, nothing will be printed. So the moral is just, when testing this, make sure you use a different warning message each time, or you'll get very confused. |
Recording this here so it doesn't get lost: Marc-Andre Lemberg suggested on python-ideas that for the builtin REPL, this should be enabled iff sys.stdin.isatty(). (I guess he is worried about 'cat script.py | python'.) I'm not really sure whether this falls on the useful or confusing side of the line myself. |
I gave that a shot. |
For whatever it's worth as a non-core-developer, the patch looks good to me. |
Ping. I know this is pretty trivial and everyone's focused on 3.5-related stuff, but it would be nice to get this finalized soon b/c the sooner CPython commits to some standard behavior here, the sooner all the other (faster-moving) python REPLs will converge to match. |
Note that I've signed the CLA, and it has been taken into account, as now I have a small star agains my name (if it was the limiting factor, we never know). |
I've changed the stage to "test needed". At a minimum, an interactive test should be written and added to the documentation. Better would be an automated test (perhaps via subprocess). The documentation should also be updated; at a minimum, there should be a versionchanged to say when the default changed. Ideally, also some clarification on the intended differences between DeprecationWarning and PendingDeprecationWarning, and whether there are any behavioral differences. |
Hum, working on the automated test. It is slightly annoying as with subprocess python is not in a tty, so there will be no change in behavior. I'm not sure how to proceed.
Where would that be ? In the warnings docs, or is there a specific place for "Manual tests" ? |
Instead of using python directly in a subprocess, try calling a shell command that in turns calls python. (Admittedly, this may look like the pipe scenario...) In theory, you could even drive another python interactively, using a GUI runner, but I'm not sure how much new infrastructure that would add to the test suite, so it might not be worth it. I'm not aware of a list of manual tests (though perhaps there *should* be one...); I would put it under the DeprecationWarning docs, sort of like an example. Others may have better ideas. |
I don't see how any of those suggestions help for writing an automated test. Spawning a shell is irrelevant; the problem is to get a tty, which is much harder. There only way I can see that might work for an automated test is to use (isatty(sys.stdin) does do something sensible on Windows, right? I hope?) |
Would
Otherwise, I can also (try to) implement a debug flag "--FakeTTYYesIMReallySure" that make |
If pty is going to work at all then it should work regardless of whether the tests themselves are being run under a tty, yes. I personally would not want to merge a test based on making isatty lie, because the point of tests is to increase confidence that things work, and that test sounds more complicated and error prone than the original code being tested. I'm not a cpython committer though... I am more concerned that isatty might actually just not work on windows, since it is actually strictly speaking true that you never have a "try" on Windows, even if you might have a gui console window open. It would be a good idea to check this quickly... The other option would be to just remove the isatty check from the patch entirely -- it wasn't something that had strong consensus in the first place. |
Astonishingly isatty appear to work on windows: http://bugs.python.org/issue18553 Up to core python for istty(). |
On windows, when python is started from the command line without a GUI, os.isatty(sys.stdin) raises an error, but os.isatty(sys.stdin.fileno()) returns true. Within IDLE, os.isatty(sys.stdin.fileno()) also raises an error, but os.isatty(0), os.isatty(1), and os.isatty(2) all return true. I'm not sure exactly when you want which warning to show, let alone whether the above tests are relevant, but I'm happy to run some tests if you can tell me what you're looking for. |
Hi, just to say - I'm happy to help steer this through. I think its an important ecosystem fixup. |
On testing this - I don't think subprocess tests are necessarily needed. The scenarios are these (from Nick's comment):
Interactive REPL. This includes IDLE in my opinion.
Everything else. On docs: yes, we should update the docs for:
|
As Robert suggests, I think it's OK to issue the deprecation warnings for code run via "python < script.py" or "cat script.py | python". Reverting to the current behaviour if folks actually want that would just be a matter of passing the file in directly, rather than piping it via stdin: "python script.py". That eliminates any need to care about whether the input is a tty or not, and we can just focus on the code paths. If you grep the CPython code base for "ps1" you're likely to find most of the relevant places ("sys.ps1" hold the interactive prompt, while "sys.ps2" holds the continuation prompt) |
ok, thanks. I'll remove the is tty ans push docs changes on a new patch. Thanks ! |
I'm not sure that is acceptable. "Just changing it to a file" could break an application's structure that is depending on being able to use stdin to run scripts. As one example, vim scripts can embed python code...now, what VIM does behind the scenes with that I don't know, and it might be able to write it out to a file or put somewhere in memory for the embedded interpreter to run, but the point is that here is a an application whose API is embedded scripts that get fed into python, not independent files that python gets called on. I'm guessing there are other applications out there that *would* be affected by this, even if vim itself isn't (and I don't know if it is or not). |
# unittest (Pending)DeprecationWarning are already enabled if no -W flags are given. # doctest We enable the DW in REPL only if originate from Should the doctest fail if such a warning is raised, or actually just print it to the output, letting the doctest pass ? |
The only way for this change to *break* something is if:
I think we're far more likely to break something else messing about with TTY detection, than we are keeping things as simple as we can and saying that if folks are running code as if they're a human rather than like a computer, then they're going to get the same deprecation warnings we want a human to see. *If* we get significant bug reports about that during the 3.6 alpha/beta cycle, *then* we can potentially consider limiting it to cases with an actual TTY. I just don't want us to borrow trouble and make this unnecessarily hard to test in the process. |
Matthias: just print it and let the doctest pass. That's consistent with how unittest works (unless warnings have been turned into errors, in which case it should fail, but I don't think you have to do anything to make that happen). Nick: OK. It is true that we aren't going to break anything if we get this wrong, just mildly annoy people :) |
A while ago I proposed to document our deprecation process and the use of PDW/DW in a PEP: There I suggest that we (CPython) stop using PDW but leave it around for other projects to use as they see fit. I'm not sure CPython should do anything special to distinguish PDW from DW. |
@mbussonn - I don't see an updated non-tty-checking patch from you? |
Sorry for the delay, trying to get back on this. Please find attached a new patch that does not check whether is is a tty. |
See also PEP-565. |
If you consider that the REPL is designed for developers, I would also suggest to show ResourceWarning by default. I'm not sure of that since I like to write crappy code of REPL (and I hope that nobody logs my keyboard)! Example: haypo@selma$ python3
Python 3.6.2 (default, Oct 2 2017, 16:51:32)
>>> open("/etc/issue").read()
'\\S\nKernel \\r on an \\m (\\l)\n\n' I may be annoyed by a ResourceWarning warning here, since it's a oneliner written to be only run once. I know that my code is crappy, but I also know that it works well and it's much shorter to write than "with open(..) as fp: fp.read()" :-) Yet another approach: I proposed to add a "developer mode", -X dev option: |
I don't think anybody consistently does proper resource management in the REPL, so the trade-offs involved there are quite different from those for deprecation warnings. Assuming PEP-565 gets accepted, we'll end up with once-per-session warnings for use of deprecated APIs (due to the REPL's perpetually reset line counter, as per bpo-1539925), and that seems like a good level of notification to me (not too noisy, not so unobtrusive as to never be seen at all) |
If it's of any interest to this discussion, for SymPy (for some time) we have used a custom subclass of DeprecationWarning that we enable by default https://github.com/sympy/sympy/blob/master/sympy/utilities/exceptions.py. I don't know if there are major libraries that do something similar. Our reasoning is that we really do want everybody to see the warnings. Obviously direct users of SymPy (both interactive users and library developers) need to see them so they can fix their code. But also if library X uses a deprecated behavior and a user of library X sees a deprecation warning for SymPy inside of library X, that incentivises them to bug the library X developers to fix the behavior (or PR it). The whole point of warnings as we see it is to be as loud as possible while still keeping things working, to avoid the situation where things stop working (when the deprecated behavior is removed). And +<however many points I'm allowed to have> to Nathaniel's point that DeprecationWarnings are about more than just the standard library. Tons of libraries use the built in warnings, and the default warnings behavior makes no distinction between warnings coming from the standard library and warnings coming from other places. |
After implementing PEP-565, are there reasons to keep this issue open? |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: