classification
Title: Fix division in tabnanny
Type: behavior Stage: resolved
Components: Demos and Tools, Library (Lib) Versions: Python 3.2, Python 3.3, Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ezio.melotti Nosy List: ezio.melotti, georg.brandl, jcea, python-dev, serhiy.storchaka
Priority: normal Keywords: easy, patch

Created on 2012-11-15 15:46 by serhiy.storchaka, last changed 2012-12-14 02:41 by jcea. This issue is now closed.

Files
File name Uploaded Description Edit
tabnanny_div.patch serhiy.storchaka, 2012-11-15 15:46 review
Messages (7)
msg175622 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-11-15 15:46
Tabnanny should use floor division in calculation, the comment says about this. But in the current Python 3 code "/" was not changed to "//".
msg175669 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-11-16 07:30
Well, sounds reasonable since we're working with an integer number of spaces :)
msg175677 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2012-11-16 10:49
The patch looks OK, buy can you provide a way to reproduce the error (if you get any)?
Should we add tests for tabnanny?
I tried to get an error from tabnanny but the only thing I got was a ResourceWarning (that can be easily fixed by a finally: f.close() near the end of the check() method).
I also noticed that tabnanny doesn't detect any problem with:
if 0:
   \t\t\t\t\t\t\tprint(1)
else:
    print(2)
    if 1:
       print()
    else:
   \t  \t     print()
msg175678 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-11-16 11:07
> The patch looks OK, buy can you provide a way to reproduce the error (if you get any)?

No, I have not any. I am even not sure tabnanny works at all. But this bug is obvious. So, let's fix it and go on.

> Should we add tests for tabnanny?

This will be good, but it is a different issue. I'm not ready to write a test.

> I tried to get an error from tabnanny but the only thing I got was a ResourceWarning (that can be easily fixed by a finally: f.close() near the end of the check() method).

I can't see a ResourceWarning. A "finally: f.close()" already exists near the end of the check() method.
msg175679 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2012-11-16 11:11
> I can't see a ResourceWarning. A "finally: f.close()" already
> exists near the end of the check() method.

Looks like it was added in 3.3.
msg175680 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-11-16 11:18
New changeset fc17fdd42c66 by Ezio Melotti in branch '3.2':
#16478: use floor division in tabnanny and fix a ResourceWarning.  Patch by Serhiy Storchaka.
http://hg.python.org/cpython/rev/fc17fdd42c66

New changeset d7558e4015a4 by Ezio Melotti in branch '3.3':
#16478: merge with 3.2.
http://hg.python.org/cpython/rev/d7558e4015a4

New changeset 377a50f8cb8b by Ezio Melotti in branch 'default':
#16478: merge with 3.3.
http://hg.python.org/cpython/rev/377a50f8cb8b
msg175681 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2012-11-16 11:19
Applied the patch on all 3 branches and fixed the resource warning in 3.2.
Thanks for the report and the patch!
History
Date User Action Args
2012-12-14 02:41:08jceasetnosy: + jcea
2012-11-16 11:19:41ezio.melottisetstatus: open -> closed
messages: + msg175681

assignee: ezio.melotti
resolution: fixed
stage: patch review -> resolved
2012-11-16 11:18:11python-devsetnosy: + python-dev
messages: + msg175680
2012-11-16 11:11:22ezio.melottisetmessages: + msg175679
2012-11-16 11:07:16serhiy.storchakasetmessages: + msg175678
2012-11-16 10:49:56ezio.melottisetnosy: + ezio.melotti
messages: + msg175677
2012-11-16 07:30:19georg.brandlsetmessages: + msg175669
2012-11-15 22:13:28pitrousetnosy: + georg.brandl
2012-11-15 15:46:58serhiy.storchakacreate