Skip to content
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

gh-102498 Clean up unused variables and imports in the email module #102482

Merged
merged 16 commits into from
Apr 24, 2023

Conversation

JosephSBoyle
Copy link
Contributor

@JosephSBoyle JosephSBoyle commented Mar 6, 2023

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

@warsaw
Copy link
Member

warsaw commented Mar 7, 2023

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.

@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 JosephSBoyle force-pushed the fix-unused-vars-in-email branch from 2c31a05 to dc7176e Compare March 7, 2023 15:31
@hugovk
Copy link
Member

hugovk commented Mar 7, 2023

To appease Bedevere about the issue number, please prefix the PR title as "gh-XXXXX:"

@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
Copy link
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.

@terryjreedy
Copy link
Member

If you used a program/linter to find these, please add 'found by xxxx' to the first sentence in the message.

@hugovk
Copy link
Member

hugovk commented Mar 7, 2023

Also an unused msg here (found by Flake8):

msg = self._pop_message()

@JosephSBoyle
Copy link
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:)

@AlexWaygood
Copy link
Member

AlexWaygood commented Mar 7, 2023

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 :)

@AlexWaygood
Copy link
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:

@JosephSBoyle JosephSBoyle force-pushed the fix-unused-vars-in-email branch from aa6402a to 76f5222 Compare March 8, 2023 11:01
@JosephSBoyle
Copy link
Contributor Author

JosephSBoyle commented Mar 9, 2023

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:)

@hugovk
Copy link
Member

hugovk commented Mar 9, 2023

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

@JosephSBoyle
Copy link
Contributor Author

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

@warsaw warsaw enabled auto-merge (squash) April 24, 2023 18:44
@warsaw warsaw merged commit 04ea048 into python:main Apr 24, 2023
carljm added a commit to carljm/cpython that referenced this pull request Apr 24, 2023
* 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
* 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants