GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-29919: Remove unused imports found by pyflakes by vstinner · Pull Request #137 · python/cpython

Conversation

@vstinner

@vstinner vstinner commented Feb 16, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Make also minor PEP8 coding style fixes on modified imports.

http://bugs.python.org/issue29919

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Feb 16, 2017
orsenthil
orsenthil approved these changes Feb 16, 2017
View reviewed changes
serhiy-storchaka
serhiy-storchaka requested changes Feb 16, 2017
View reviewed changes

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please open an issue on the tracker. This is enough large change.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Lib/tkinter/test/test_tkinter/test_widgets.py Outdated

@serhiy-storchaka serhiy-storchaka Feb 16, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setUpModule should be imported in the module namespace!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@vstinner vstinner Mar 27, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Lib/tkinter/test/test_ttk/test_widgets.py Outdated

@serhiy-storchaka serhiy-storchaka Feb 16, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@vstinner vstinner Mar 27, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Lib/unittest/__main__.py Outdated

@serhiy-storchaka serhiy-storchaka Feb 16, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks suspicious. Are you sure that TestProgram is not needed?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@vstinner vstinner Feb 16, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lib/unittest/main.py ends with "main = TestProgram". And TestProgram is not used in Lib/unittest/main.py.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Lib/idlelib/idle_test/test_searchbase.py Outdated

@serhiy-storchaka serhiy-storchaka Feb 16, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why BooleanVar and StringVar are commented out?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@vstinner vstinner Feb 16, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are used in commented code a few lines below. I don't want to remove this commented code, since I don't know this file.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@yan12125 yan12125 mentioned this pull request Feb 16, 2017
bpo-29442: Replace optparse with argparse in setup.py #139
Merged
@orsenthil

orsenthil commented Feb 16, 2017

Copy link
Copy Markdown
Member

Here was my basis for approval. The change is an automated one using pyflakes and our unit-tests are successful. I could not spot anything critical after looking at the diff. ( That is, if things would have broken by removal of top-level imports, then we should have seen when the module was invoked).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@warsaw

warsaw commented Feb 17, 2017

Copy link
Copy Markdown
Member

Gosh I'd love to enable flake8 on the stdlib, since it often causes my eyes to bleed red when I open a py file from the stdlib. But I suppose that will just cause unnecessary code churn. It's still nice to clean things up when you can, like unused imports.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner

vstinner commented Feb 18, 2017 via email

Copy link
Copy Markdown
Member Author
2017-02-18 0:35 GMT+01:00 Barry Warsaw <[email protected]>:
Gosh I'd love to enable flake8 on the stdlib, since it often causes my eyes to bleed red when I open a py file from the stdlib. But I suppose that will just cause unnecessary code churn. It's still nice to clean things up when you can, like unused imports.
Technically, it's possible, but you should expect a lot of "# noqa" to ignore warnings when pyflakes is not smart enough to understand "Python magic". For example, even with my patch, there are still tons of "unused import" warnings.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@serhiy-storchaka serhiy-storchaka added the type-feature A feature request or enhancement label Feb 18, 2017
@vstinner vstinner changed the title Remove unused imports found by pyflakes bpo-29919: Remove unused imports found by pyflakes Mar 27, 2017
@vstinner
bpo-29919: Remove unused imports found by pyflakes
debbb91
Make also minor PEP8 coding style fixes on modified imports.
@vstinner

vstinner commented Mar 27, 2017

Copy link
Copy Markdown
Member Author

I rebased my patch and removed the two changes removing setUpModule() :-)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner

vstinner commented Mar 27, 2017

Copy link
Copy Markdown
Member Author

@serhiy-storchaka: "Please open an issue on the tracker. This is enough large change."

I don't see the value added by creating an issue, but ok, I created http://bugs.python.org/issue29919 :-)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

serhiy-storchaka
serhiy-storchaka approved these changes Mar 27, 2017
View reviewed changes
@vstinner
vstinner merged commit d6debb2 into python:master Mar 27, 2017
@vstinner

vstinner commented Mar 27, 2017

Copy link
Copy Markdown
Member Author

Thanks for the reviews!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner
vstinner deleted the unused_imports branch March 27, 2017 14:05
terryjreedy added a commit that referenced this pull request Jun 11, 2017
@terryjreedy
[3.6] bpo-29919 Remove unused imports from idlelib [GH-137] (#2105)
c6696fe
Part of patch by Victor Stinner.
(cherry-pick from d6debb2)
akruis pushed a commit to akruis/cpython that referenced this pull request Oct 22, 2017
Anselm Kruis
Stackless issue python#137: Add Stackless support to "call" functions
6e3bb41
Add Stackless support to all call-API-functions in abstract.c
akruis pushed a commit to akruis/cpython that referenced this pull request Oct 22, 2017
Anselm Kruis
Stackless issue python#137: Add Stackless support to "call" functions
d342469
Add Stackless support to _PyObject_FastCallKeywords() and
_PyFunction_FastCallKeywords.
akruis pushed a commit to akruis/cpython that referenced this pull request Oct 29, 2017
Anselm Kruis
Stackless issue python#137: Add Stackless support to "call" functions
9f86886
Add Stackless support to _PyObject_Call_Prepend().
akruis pushed a commit to akruis/cpython that referenced this pull request Oct 29, 2017
Anselm Kruis
Stackless issue python#137: Add Stackless support to "call" functions
3131f57
- Add Stackless support to _PyObject_FastCallKeywords().
- Use STACKLESS_PROPOSE_... in a consistent way.
akruis pushed a commit to akruis/cpython that referenced this pull request Oct 29, 2017
Anselm Kruis
Stackless issue python#137: Add Stackless support to "call" functions
df3f77f
Add Stackless support to _PyCFunction_FastCallKeywords().
akruis pushed a commit to akruis/cpython that referenced this pull request Oct 29, 2017
Anselm Kruis
Stackless issue python#137: Add Stackless support to "call" functions
f590d9d
Add Stackless support for METH_FASTCALL C-functions.
Add a warning about the ABI change to the Stackless changelog.
akruis pushed a commit to akruis/cpython that referenced this pull request Oct 29, 2017
Anselm Kruis
Stackless issue python#137: Add Stackless support to "call" functions
4c4828a
Add documentation and a changelog entry.
akruis pushed a commit to akruis/cpython that referenced this pull request Mar 25, 2018
Anselm Kruis
Stackless issue python#137: Add Stackless support to "call" functions
91a1f8b
Add Stackless support to _PyObject_CallFunctionVa() and related
functions.
akruis pushed a commit to akruis/cpython that referenced this pull request Mar 25, 2018
Anselm Kruis
Stackless issue python#137: Add Stackless support to "call" functions
459f1e3
Fix Stackless support of _PyCFunction_FastCallKeywords().
akruis pushed a commit to akruis/cpython that referenced this pull request Mar 25, 2018
Anselm Kruis
Stackless issue python#137: Add Stackless support to "call" functions
dea7004
Add Stackless support to _PyMethodDescr_FastCallKeywords().
akruis pushed a commit to akruis/cpython that referenced this pull request Mar 25, 2018
Anselm Kruis
Stackless issue python#137: Add Stackless support to "call" functions
f0a8465
Add Stackless support to cfunction_call_varargs().
akruis pushed a commit to akruis/cpython that referenced this pull request Mar 25, 2018
Anselm Kruis
Stackless issue python#137: Add Stackless support to "call" functions
06b7237
Add Stackless support to function _PyObject_FastCall_Prepend().
jaraco pushed a commit that referenced this pull request Dec 2, 2022
@pyup-bot @Mariatta
Update pytest from 3.7.2 to 3.7.3 (#137)
2e180b3
SonicField added a commit to SonicField/cpython that referenced this pull request Apr 25, 2026
@SonicField
builder: BCOffset/InstrIndex named conversions + 3-class fix in build…
8e0c73e
…_inline_except_opcode_array_c

Fixes THREE boundary-domain bugs in build_inline_except_opcode_array_c
(introduced W27c #2a 7135d94), found across two HIR-diff Phase 0 cycles
of the W-2B-RECONVERT investigation.

CONVENTION (Python/jit/bytecode.cpp:8-14, builder.cpp:1235,
phx_frame_state.h cur_instr_offs semantics):
- jit_bc_instr_init expects INSTRUCTION INDEX (codeUnit[])
- jit_bc_instr_get_jump_target / next_offset / base_offset return whatever
  was stored at init (now INDEX after Class A fix)
- phx_block_map keys are BYTE OFFSETS
- OpcodeArrayEntry.base_offset is consumed downstream as BYTE OFFSET
  (cur_instr_offs assignment per builder_emit_c.c:3320)
- BCOffset.value() (caller-passed except_body_offset) is BYTE OFFSET
- BYTES = INDEX * sizeof(_Py_CODEUNIT) (= 2 in 3.12)

NAMED CONVERSIONS (Python/jit/bytecode_c.h, gated by _Py_OPCODE):
  static inline int phx_bc_offset_to_instr_index(int byte_off);
  static inline int phx_bc_instr_index_to_offset(int instr_idx);

Codifies the boundary-domain rule by example (per pythia python#137 python#2 +
supervisor 19:01:19Z + theologian 19:01:17Z).

THREE FIXES:

CLASS A (line 3241): jit_bc_instr_init was passed except_body_offset
(BCOffset.value() byte offset) where INSTRUCTION INDEX was expected.
codeUnit(code)[byte_offset] read PAST end of co_code → garbage opcode →
switch-default → Deopt with corrupt frame state. Found by Phase 0
HIR-diff (test_exc_raise_catch bb 12: correct Return -1 vs corrupt
LoadConst NoneType + Deopt at offset 58).

CLASS B (line 3273-3275, theologian class-of-bug audit 18:42:53Z):
target = jit_bc_instr_get_jump_target returns INDEX, but
phx_block_map_lookup_or_panic expects BYTE OFFSET. Without conversion,
JUMP_BACKWARD-in-except-body lookup fails: JIT_CHECK_C panic OR silent
wrong-block. Dormant pre-fix because no test had backward-jump-in-except-body.

CLASS C (line 3260, exposed by Phase 0' HIR-diff after Class A+B fix):
After Class A fix corrected the init to INDEX, jit_bc_instr_base_offset
returns INDEX. But entry->base_offset is consumed downstream (line 3320)
as BYTE OFFSET via match_tc.frame.cur_instr_offs assignment. Pre-fix
'correct by accident' — Class A's BYTES-as-INDEX init wrote BYTES into
bci->base_offset, so jit_bc_instr_base_offset returned BYTES, matching
downstream. Correct Class A exposed Class C: cur_instr_offs got INDEX
(half the correct BYTE value) → interpreter Deopt resumed at wrong
bytecode position → SIGSEGV in test_multiple_exceptions_in_loop
(deterministic 0/20 post Class A+B fix, vs 20/20 PASS pre-W27c).

DIAGNOSIS:
HIR-diff for test_multiple_exceptions_in_loop revealed Deopt CurInstrOffset
124 (correct, BYTES) → 62 (wrong, INDEX = 124/2). Direct evidence of the
domain mismatch.

LATENT in pushed W27c #2a (e4e7507 on SonicField/cpython): all three
classes present. Class A, B dormant (no test exercises emitInlineExceptionMatch
or JUMP_BACKWARD-in-except-body). Class C compensated by Class A — both
broken in opposite directions canceling out for downstream consumers of
entry->base_offset. ALL three must fix together.

INVESTIGATION CHAIN:
- testkeeper bisect 17:52:30Z localized #2b regression → W27c #2b sole
- pythia python#136 python#1 18:23:34Z flagged HEAP/RACE rode on absence-of-evidence
- generalist 18:24Z proposed HIR-diff Phase 0 falsifier
- generalist 18:32+18:34Z captured HIR_2a + HIR_2b dumps; found Class A
- theologian 18:42:53Z class-of-bug audit found Class B
- supervisor 18:43:17Z directed dual-fix
- pythia python#137 python#2 19:00:29Z flagged inline-arithmetic violates boundary-domain rule
- supervisor 19:01:19Z + theologian 19:01:17Z directed amend to named conversions
- testkeeper 19:06:55Z full Phoenix gate caught NEW regression
  (test_multiple_exceptions_in_loop deterministic 0/20)
- generalist 19:14Z HIR-diff Phase 0' on test_multiple_exceptions_in_loop
  revealed Class C (cur_instr_offs 124→62)
- supervisor 19:16:42Z authorized Class C fix + extended audit

OTHER OpcodeArrayEntry FIELDS AUDITED (per supervisor 19:16:42Z extended
class-of-bug discipline):
- entry->opcode: written from jit_bc_instr_opcode (no domain — opcode value);
  consumed in dispatch loop switch. CLEAN.
- entry->oparg: written from jit_bc_instr_oparg (no domain — oparg value);
  consumed in dispatch loop emit calls. CLEAN.
- entry->base_offset: Class C above; FIXED.
- entry->const_obj: written from PyTuple_GET_ITEM (PyObject*); consumed in
  dispatch loop hir_type_from_object. CLEAN (no domain conversion).
- entry->jump_target_block: Class B above; FIXED.

VERIFICATION pending (testkeeper 4-suite extended verify):
1. 30x test_exc_raise_catch (Class A regression)
2. 30x test_exc_binary_subscr_dict_in_try (Class A latent activation)
3. 30x test_exc_continue_in_loop (Class B latent activation)
4. 30x multi-except-in-loop sentinel (Class C latent activation)
5. Full Phoenix suite (which originally caught Class C)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@orsenthil orsenthil orsenthil approved these changes
@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

type-feature A feature request or enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@vstinner @orsenthil @warsaw @serhiy-storchaka @the-knights-who-say-ni
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.