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 — gh-102498 Clean up unused variables and imports in the email module by JosephSBoyle · Pull Request #102482 · python/cpython

Conversation

@JosephSBoyle

@JosephSBoyle JosephSBoyle commented Mar 6, 2023 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

Found some unused vars using the following pyflakes command $ python3.12 -m pyflakes Lib/email/. Inspired by @AlexWaygood 's use of the tool to find issues in the test_typing.py file (can't seem to find the exact issue).

Cleanup some unused variables and imports in the email module.

In text.py I replaced a pagebreak character (U+000C) with a newline. The latter doesn't seem to be rendered by GH. VScode shows it, as will most editors I imagine.

GH Issue: #102498

  • Issue: Clean up unused variables and imports in the email module #102498

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@JosephSBoyle
JosephSBoyle requested a review from a team as a code owner March 6, 2023 21:48
@bedevere-bot bedevere-bot added the awaiting review label Mar 6, 2023
@AlexWaygood AlexWaygood added skip news topic-email labels Mar 6, 2023
@warsaw

warsaw commented Mar 7, 2023

Copy link
Copy Markdown
Member

Thanks for the cleanup. I can review it in more detail after a few housekeeping updates have been made (updating the branch to current HEAD, creating an issue, and adding an issue number).

You can just delete the page feed line (see PEP 8, two blank lines only between top level constructs). The page feeds are a historical holdover for when this library was developed independently.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@JosephSBoyle JosephSBoyle mentioned this pull request Mar 7, 2023
Clean up unused variables and imports in the email module #102498
Closed
@JosephSBoyle JosephSBoyle changed the title Clean up unused variables and imports in the email module Clean up unused variables and imports in the email module #102498 Mar 7, 2023
@JosephSBoyle
Clean up unused variables and imports in the email module
dc7176e
@JosephSBoyle
JosephSBoyle force-pushed the fix-unused-vars-in-email branch from 2c31a05 to dc7176e Compare March 7, 2023 15:31
@JosephSBoyle
Remove extra newline char
d78691f
@hugovk

hugovk commented Mar 7, 2023

Copy link
Copy Markdown
Member

To appease Bedevere about the issue number, please prefix the PR title as "gh-XXXXX:"
JosephSBoyle reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@JosephSBoyle JosephSBoyle changed the title Clean up unused variables and imports in the email module #102498 gh-102498 Clean up unused variables and imports in the email module Mar 7, 2023
@JosephSBoyle

JosephSBoyle commented Mar 7, 2023

Copy link
Copy Markdown
Contributor Author

Hi @warsaw, thanks for the quick response. I've added an issue, rebased to the current HEAD and removed that extra line as you requested. Should be good for a review now.

@hugovk cheers for the headsup r.e the issue number.
hugovk reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

terryjreedy commented Mar 7, 2023

Copy link
Copy Markdown
Member

If you used a program/linter to find these, please add 'found by xxxx' to the first sentence in the message.
JosephSBoyle reacted with thumbs up emoji JosephSBoyle reacted with rocket emoji
All reactions
  • 👍 1 reaction
  • 🚀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@JosephSBoyle
Remove superflous dict+unpacking syntax
8cda6c2
@hugovk

hugovk commented Mar 7, 2023

Copy link
Copy Markdown
Member

Also an unused msg here (found by Flake8):

cpython/Lib/email/feedparser.py

Line 269 in 4a3ea1f

msg = self._pop_message()

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@JosephSBoyle
Remove unused 'msg' var
aa6402a
@JosephSBoyle

JosephSBoyle commented Mar 7, 2023

Copy link
Copy Markdown
Contributor Author

There are 45 pagebreaks remaining in the entire cpython repo, 34 of which are in email. I'm happy to replace them with newlines / delete them as appropriate per pep-8 if people want them gone. Might be something for another issue though:)
All reactions

Sorry, something went wrong.

Uh oh!

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

AlexWaygood commented Mar 7, 2023 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

There are 45 pagebreaks remaining in the entire cpython repo, 34 of which are in email. I'm happy to replace them with newlines / delete them as appropriate per pep-8 if people want them gone. Might be something for another issue though:)

I agree with @warsaw that it's now time for these to go, but it would be better to remove all of them from the stdlib at once, in a separate PR, I think, rather than in this PR.

(Not requesting a change to this PR; it's fine to remove the one you're already removing, in my opinion, since you're already touching that file :)
All reactions

Sorry, something went wrong.

Uh oh!

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

AlexWaygood commented Mar 7, 2023

Copy link
Copy Markdown
Member

Inspired by @AlexWaygood 's use of the tool to find issues in the test_typing.py file (can't seem to find the exact issue).

This is the one:

  • Several minor bugs in test_typing highlighted by pyflakes #102444
JosephSBoyle reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@JosephSBoyle JosephSBoyle mentioned this pull request Mar 7, 2023
Remove invisible pagebreak chars from the standard library #102507
Closed
JosephSBoyle added 4 commits March 8, 2023 11:01
@JosephSBoyle
Clean up unused variables and imports in the email module
e579c75
@JosephSBoyle
Remove extra newline char
f96ae11
@JosephSBoyle
Remove superflous dict+unpacking syntax
d2f40af
@JosephSBoyle
Remove unused 'msg' var
76f5222
@JosephSBoyle
JosephSBoyle force-pushed the fix-unused-vars-in-email branch from aa6402a to 76f5222 Compare March 8, 2023 11:01
JosephSBoyle and others added 3 commits March 8, 2023 14:11
@JosephSBoyle
Merge branch 'main' into fix-unused-vars-in-email
b2f6548
@JosephSBoyle
Merge branch 'fix-unused-vars-in-email' of github.com:JosephSBoyle/cp…
45fa813 
…ython into fix-unused-vars-in-email
@JosephSBoyle
Merge remote-tracking branch 'upstream/main' into fix-unused-vars-in-…
71e6e58 
…email
@JosephSBoyle

JosephSBoyle commented Mar 9, 2023 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

I appear to have duplicated the commits during the merge somehow. I would rebase and drop them but I'm under the impression that force-pushing is discouraged. If we merge this PR they'll all be squashed anyways as I understand it.. Let me know if dropping the duplicated commits is preferable and I can do that:)
All reactions

Sorry, something went wrong.

Uh oh!

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

hugovk commented Mar 9, 2023

Copy link
Copy Markdown
Member

Correct, all PRs are squashed and merged, so don't worry about the individual commits too much. 👍
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
JosephSBoyle added 4 commits March 27, 2023 09:34
@JosephSBoyle
Merge branch 'main' into fix-unused-vars-in-email
3e781bf
@JosephSBoyle
Merge branch 'main' into fix-unused-vars-in-email
8a5e896
@JosephSBoyle
Merge branch 'main' into fix-unused-vars-in-email
f8f97e1
@JosephSBoyle
Merge branch 'main' into fix-unused-vars-in-email
40a3ad9
@JosephSBoyle

JosephSBoyle commented Apr 22, 2023

Copy link
Copy Markdown
Contributor Author

Hi @warsaw. Could you please take a look at this when you get a chance?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
warsaw
warsaw approved these changes Apr 24, 2023
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels Apr 24, 2023
@warsaw
Merge branch 'main' into fix-unused-vars-in-email
1cc123f
@warsaw
warsaw enabled auto-merge (squash) April 24, 2023 18:44
@warsaw
warsaw merged commit 04ea048 into python:main Apr 24, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Apr 24, 2023
carljm added a commit to carljm/cpython that referenced this pull request Apr 24, 2023
@carljm
Merge branch 'main' into superopt
19b8025 
* main: (53 commits)
  pythongh-102498 Clean up unused variables and imports in the email module  (python#102482)
  pythongh-99184: Bypass instance attribute access in `repr` of `weakref.ref` (python#99244)
  pythongh-99032: datetime docs: Encoding is no longer relevant (python#93365)
  pythongh-94300: Update datetime.strptime documentation (python#95318)
  pythongh-103776: Remove explicit uses of $(SHELL) from Makefile (pythonGH-103778)
  pythongh-87092: fix a few cases of incorrect error handling in compiler (python#103456)
  pythonGH-103727: Avoid advancing tokenizer too far in f-string mode (pythonGH-103775)
  Revert "Add tests for empty range equality (python#103751)" (python#103770)
  pythongh-94518: Port 23-argument `_posixsubprocess.fork_exec` to Argument Clinic (python#94519)
  pythonGH-65022: Fix description of copyreg.pickle function (python#102656)
  pythongh-103323: Get the "Current" Thread State from a Thread-Local Variable (pythongh-103324)
  pythongh-91687: modernize dataclass example typing (python#103773)
  pythongh-103746: Test `types.UnionType` and `Literal` types together (python#103747)
  pythongh-103765: Fix 'Warning: py:class reference target not found: ModuleSpec' (pythonGH-103769)
  pythongh-87452: Improve the Popen.returncode docs
  Removed unnecessary escaping of asterisks (python#103714)
  pythonGH-102973: Slim down Fedora packages in the dev container (python#103283)
  pythongh-103091: Add PyUnstable_Type_AssignVersionTag (python#103095)
  Add tests for empty range equality (python#103751)
  pythongh-103712: Increase the length of the type name in AttributeError messages (python#103713)
  ...
carljm added a commit to carljm/cpython that referenced this pull request Apr 24, 2023
@carljm
Merge branch 'superopt' into superopt_spec
1972ca4 
* superopt: (82 commits)
  pythongh-101517: fix line number propagation in code generated for except* (python#103550)
  pythongh-103780: Use patch instead of mock in asyncio unix events test (python#103782)
  pythongh-102498 Clean up unused variables and imports in the email module  (python#102482)
  pythongh-99184: Bypass instance attribute access in `repr` of `weakref.ref` (python#99244)
  pythongh-99032: datetime docs: Encoding is no longer relevant (python#93365)
  pythongh-94300: Update datetime.strptime documentation (python#95318)
  pythongh-103776: Remove explicit uses of $(SHELL) from Makefile (pythonGH-103778)
  pythongh-87092: fix a few cases of incorrect error handling in compiler (python#103456)
  pythonGH-103727: Avoid advancing tokenizer too far in f-string mode (pythonGH-103775)
  Revert "Add tests for empty range equality (python#103751)" (python#103770)
  pythongh-94518: Port 23-argument `_posixsubprocess.fork_exec` to Argument Clinic (python#94519)
  pythonGH-65022: Fix description of copyreg.pickle function (python#102656)
  pythongh-103323: Get the "Current" Thread State from a Thread-Local Variable (pythongh-103324)
  pythongh-91687: modernize dataclass example typing (python#103773)
  pythongh-103746: Test `types.UnionType` and `Literal` types together (python#103747)
  pythongh-103765: Fix 'Warning: py:class reference target not found: ModuleSpec' (pythonGH-103769)
  pythongh-87452: Improve the Popen.returncode docs
  Removed unnecessary escaping of asterisks (python#103714)
  pythonGH-102973: Slim down Fedora packages in the dev container (python#103283)
  pythongh-103091: Add PyUnstable_Type_AssignVersionTag (python#103095)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@warsaw warsaw warsaw approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

skip news topic-email

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.

6 participants

@JosephSBoyle @warsaw @hugovk @terryjreedy @AlexWaygood @bedevere-bot
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.